-
Notifications
You must be signed in to change notification settings - Fork 35
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
Package redesign - future of math submodule #90
Comments
Important note: this submodule is used by at least the https://github.com/diana-hep/madminer package, and we should keep the authors in the loop of the design going on here, to avoid unnecessary disruptions. FYI @johannbrehmer. |
Thanks! A move is great, but if would be awesome if you leave the LorentzVector API intact :) Let me elaborate a little bit on this. MadMiner makes heavy use of LorentzVector, not just internally, but user-facing. For instance, MadMiner users can define observables or selection cuts through expressions or functions that use LorentzVector instances. So if LorentzVector moves to a different package, that's no problem at all -- but if the LorentzVector API changes, that could break existing code that uses MadMiner. (In that case we'll have to fix the dependency to be skhep v0.5.* for the time being). Thanks a lot for listening to us! |
I know we have mentioned changes but, if any, changes to the API will be really small, and probably not in the kind of methods you use. Also, we could even imagine to keep, for at least the first release as new package, a duplication of functionality with the old signatures, if better. We will update this as the job gets done :-). |
Hi @johannbrehmer, I think it is time to get back to this now that the CC'ing @henryiii and @jpivarski. Feel free to add anyone relevant to the discussions. |
As you can see from #140 I'm adding |
Hello @Saransh-cpp, I see that you remain super active in the Vector package. This metapackage could be seen as a predecessor of Vector for various things in Scikit-HEP, until we evolved for a better and more modular vision. As you can see from this task and the repo, at this stage only the math submodule remains, aside what really is otherwise a metapackage. Could you kindly have a look at the math module to identify if anything is not superseded by Vector at this point? If the case then I can remove math and finally close this very old task. Let me know if you can help. Thanks! |
Hi @eduardo-rodrigues, thanks for the tag! I am planning to work on this issue this week. |
I discussed this issue with @jpivarski and it should be okay to remove the math submodule in this repository. The module has some functionalities that vector lacks but there are no plans to implement them in vector (intentionally). Furthermore, given that the module is not actively maintained, it might have unnoticed bugs in it and we should not recommend users to use the module. |
Thank you both. Just to be clear, did you also look at things such as the math/isclose.py and math/numeric.py modules? Nothing interesting to port there? I was also worried that we would look what we have in math/geometry.py for lines and plans. |
Vector doesn't have the equivalent of extended objects like lines and planes. It seems like a natural extension, but it's one that has never been requested. It's the sort of thing you'd likely need for a detector geometry description—and in fact, a detector geometry would need much more than just lines and planes—for which there are other Python libraries, to work with geometry in Geant4's XML language. (There was a talk on that at one of the PyHEPs, I think.) |
Ah, nice, I had not appreciated that. Good to know. Indeed lines and planes are a bit on the edge, beyond the strict scope of Vector and more into what you would need for geometry. |
As part of a major redesign, see #86, we need to discuss where the
math
submodule could/should go. The module contains classes to deal with vectors, Lorentz vectors, functions relevant toe kinematics, etc.We've had discussions on a package for a fast Pythonic implementation of vector-related classes, so that's 100% connected with this PR. And we want to move forward having those implementations in sync with what is in uproot-methods. CC for now @henryiii and @jpivarski.
To be continued ...
The text was updated successfully, but these errors were encountered: