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

JOML rotate ops #562

Merged
merged 8 commits into from
Aug 1, 2018
Merged

JOML rotate ops #562

merged 8 commits into from
Aug 1, 2018

Conversation

rimadoma
Copy link
Contributor

  • Adds LinAlg namespace
  • Add JOML dependency
  • Adds rotation ops for Vector3d and Vector3f
  • Adds various convenience OpMethods for the above
  • Bumps SNAPSHOT version

@rimadoma
Copy link
Contributor Author

If this PR is accepted, maybe the linalg namespace would be a good place for the op in PR #420

@ctrueden
Copy link
Member

Cool, thanks @rimadoma! Do you think it would be too general to call the namespace linear instead of linalg? What do other software libraries call it?

@rimadoma
Copy link
Contributor Author

I was thinking this name space would be for linear algebra on (simple) vectors and matrices. I think linear could easily lead to a loss of coherence, where the name space hosts ops from vastly different domains of maths and image processing. How about just calling it vecmath?

Apache commons calls their package org.apache.commons.math3.linear, but I think their situation is different, since the package is just limited to mathematics.

In any case, I see these ops eventually migrating to scijava-ops at which point the naming can be reconsidered.

@imagejan
Copy link
Member

linalg is used by numpy/scipy as well, and in MATLAB (at least in their outdated MuPAD notebooks), it seems.

I think linalg isn't such a bad choice.

@bnorthan
Copy link
Contributor

bnorthan commented Jun 27, 2018

What do other software libraries call it?

@ctrueden in the last year or so I've mostly been wrapping BLAS (Basic Linear Algebra Specification), and using a bit of ND4J (N-Dimensionsal Arrays for Java). I'd vote for calling the namespace linalg and use it to implement a subset of blas functionality in a user friendly way.

@rimadoma if you haven't seen it already there is a transform namespace that contains a rotate function. Although this namespace is specific to imglib2 structures.

I've also been working a RealTransform to apply a affine transform to an image.

Do you guys think all rotate ops should be in transform or is it OK to have a rotate in linalg ??

I also wonder if this functionality belongs in the core imagej-ops project, or should be in a separate joml-ops repo. Should imagej-ops be for imglib2 implementations, and other implementations implemented in separate projects??

@xulman
Copy link

xulman commented Jun 27, 2018

Do you guys think all rotate ops should be in transform or is it OK to have a rotate in linalg ??

Do we have/Can we have aliases in the ops world?
It is not always black-and-white to categorize stuff...

I also prefer linalg.

@rimadoma
Copy link
Contributor Author

@bnorthan All the ops in the transform namespace seem to handle RAIs, so imo adding Vector3d stuff would make it less coherent. Maybe it is worth then, like you suggested, to have a joml-ops or other such separate module, and limit imagej-ops to imglib2 things.

@ctrueden
Copy link
Member

ctrueden commented Jul 2, 2018

This imagej/imagej-ops repository needs to be chopped into multiple different repositories before year's end. I agree that putting the JOML ops into their own repository makes sense. Calling the namespace linalg sounds good, since it sounds more consistent and preferred by all. Regarding aliases, yes, ops can have any number of aliases, including aliases in different namespaces. But we should choose them carefully because we will need to support them all into the future—if we want to reclaim a particular alias later for something different, it would still break existing scripts expecting that op to be something else.

@rimadoma I am going on vacation starting Wednesday for two weeks, so will probably not get to this PR before then. But I will try to make it a priority to merge it before end of August.

@rimadoma
Copy link
Contributor Author

rimadoma commented Jul 5, 2018

What's your opinion y'all: should this linalg repository also host ojAlgo i.e. arbitrary size matrix ops as well? The consensus at least at the MPI-CBG Dresden '17 hackathon was that there's need for both JOML and ojAlgo libraries for linear algebra.

@rimadoma
Copy link
Contributor Author

rimadoma commented Jul 5, 2018

@ctrueden Enjoy your vacation! I'll start mine before you come back, and won't return until August 2.

@ctrueden
Copy link
Member

ctrueden commented Aug 1, 2018

This is a beautiful PR, so I'm going to merge it. We can deal with the modularization separately as part of the more general modularization of ImageJ Ops.

@ctrueden ctrueden merged commit c486a51 into imagej:master Aug 1, 2018
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

Successfully merging this pull request may close these issues.

5 participants