Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create In-Place Variants for new Wedges #208

Open
GeorgeR227 opened this issue Feb 28, 2024 · 4 comments
Open

Create In-Place Variants for new Wedges #208

GeorgeR227 opened this issue Feb 28, 2024 · 4 comments

Comments

@GeorgeR227
Copy link
Collaborator

PR #201 introduced new wedge operators for various dual cases but as of now these are all out-of-place operators. Mirroring the pp wedges structure, we should include wrappers to provide an in-place and an out-of-place version for dec_matrix_generate and update gensim to unlock this functionality for simulation generation.

@lukem12345
Copy link
Member

Good catch, George. Could you make a quick set of instructions for how to add such support?

@GeorgeR227
Copy link
Collaborator Author

  1. Create a wrapper that generates both the in-place and out-of-place functions and returns them as a tuple in that order. See
    function dec_pair_wedge_product(::Type{Tuple{k,0}}, sd::HasDeltaSet) where {k}
    as an example, your actual implementation may differ based what the function looks like.
  2. Add the symbols for those wedge products into the set extra_dec_operators here
    extra_dec_operators = Set([:₁⁻¹, :₀₁, :₁₀, :₁₁, :₀₂, :₂₀])
    This is just a collection of non-matrix but in-place operators.

That should be it but it's been a while since I've done this and I remember the solution to getting the wedge to work was kinda hacky. Lmk if anything goes awry.

@lukem12345
Copy link
Member

We will hold off on this feature since it would be best to introduce the in-place version of these operators at the same time we add GPU support for these operators. GPU kernels are always in-place.

@lukem12345
Copy link
Member

This issue can be addressed by the current push in CombinatorialSpaces towards kernelizing operators AlgebraicJulia/CombinatorialSpaces.jl#113 In-place operators and their GPU versions are now one-and-the-same.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants