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

[FEA] Add cross product operator #469

Open
cliffburdick opened this issue Aug 25, 2023 · 6 comments
Open

[FEA] Add cross product operator #469

cliffburdick opened this issue Aug 25, 2023 · 6 comments

Comments

@cliffburdick
Copy link
Collaborator

Should support 3D points with an arbitrary rank (Nx3)

@mfzmullen
Copy link
Contributor

I use cross products quite a bit and would be interested in implementing this. Anything I should be aware of regarding intended implementation or anything?

@cliffburdick
Copy link
Collaborator Author

Great! I think I would do the following:

  1. Copy the operator source from something like polyval.h into a new file called cross.h with the namings inside changed
  2. Take two operators as input. Have a runtime check that the last dimension has length 3 (Or 2 for bonus points if you want z=0). The other dimensions are batching. The dimensions are a bit awkward because of the 3D nature not fitting in a warp size, but it doesn't need to be optimal right now.
  3. Add unit tests for batching and non-batching cases
  4. Update docs pointing to the tests and any extra notes

@mfzmullen
Copy link
Contributor

Great thanks! I'll start on this sometime this week.

@mfzmullen
Copy link
Contributor

As a matter of convention in MatX, are assertions best defined in the constructor for a class template specialization (e.g. like in polyval.h) or in the Exec method, like in corr.h?

@cliffburdick
Copy link
Collaborator Author

In general if they're template parameters of the class they should go at the top of the class (doesn't even have to be in the ctor). In the corr.h case those are template parameters to the function so they must be in the function itself.

@mfzmullen
Copy link
Contributor

This hasn't dropped off my radar, just haven't had a chance to look at it since implementing the runtime checks.

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