Repository navigation
feat: support to_2D/3D/4D, to_p* conversions and like() in Numba - #751
Thanatos9404 wants to merge 1 commit into
Conversation
The Numba object backend only knew the to_Vector*D and generic coordinate spellings, so to_2D/to_3D/to_4D, the momentum-named conversions (to_pxpy, to_ptphietamass, ...) and like() raised a TypingError inside njit even though they work in pure Python. Register to_2D/to_3D/to_4D on the existing to_Vector*D overloads, register each coordinate change under its momentum name as well, and add a like() overload that picks the projection from the dimension of the argument's type at compile time. Fixes scikit-hep#750
Saransh-cpp
left a comment
There was a problem hiding this comment.
Hi @Thanatos9404, thanks for the PR. I unfortunately do not advocate for new contributors using AI for PRs. I use AI daily but only in the code bases I understand.
Moreover, this PR serves no purpose as any maintainer could have done this change (with or without AI). The entire point of onboarding new contributors is for them to learn the internals of a repository, which you don’t if you offload the work to AI.
I would be very happy if a maintainer creates this AI generated PR, but not if a new contributor does it. Partially also because I would like my “voluntary” reviews to benefit the repository and attract more human (and not AI) contributors.
I hope you understand. I’ll close this PR.
|
Thanks for explaining, @Saransh-cpp. That's fair, and I understand the point about learning the internals first. |
|
Hi @Thanatos9404, thanks for understanding! We are going through AI guidelines for this repository / for the org. The discussion was triggered by my comments + other activities in the org. I'll reopen this PR and wait until an AI policy is formalised (such that you are not punished unjustly for using and declaring AI usage). |
|
Thanks, that's kind of you. Happy to wait for the policy. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #751 +/- ##
==========================================
+ Coverage 88.88% 88.95% +0.06%
==========================================
Files 96 96
Lines 10783 10816 +33
==========================================
+ Hits 9585 9621 +36
+ Misses 1198 1195 -3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
AI usage: this PR is fully AI generated. An AI coding agent wrote the code, the tests and the description below, and ran the checks listed; a second AI pass reviewed the diff. I directed the work and chose to open this PR.
🤖 AI text below 🤖
Description
Fixes #750
vector.objvectors innumba.njitraised aTypingErrorfor three groups of methods that work in pure Python. All changes are insrc/vector/backends/_numba_object.py:to_2D,to_3Dandto_4Dare registered on the existingto_Vector2D/3D/4Doverloads with a secondoverload_methoddecorator, the same wayscaleandscale2Dshare one overload.add_coordinate_changeregisters the momentum spelling next to the generic one (pxpy,ptphi,pz,energy,mass;thetaandetaunchanged), giving all 20to_p*conversions. The names are built there rather than by looping over_repr_momentum_to_generic, because that mapping does not giveenergyandmassfor the temporal part.like()overload picksto_Vector2D/3D/4Dfrom the Numba type ofotherat compile time. The result keeps the class ofself, and a non-vector argument raises aTypingErrorsaying it "is not a vector.Vector", as in Python.As before, compiled conversions take no keyword arguments, so
to_3D(z=...)still only works in pure Python.Tests
test_conversion_aliases(allto_2D/3D/4Dandto_p*names) andtest_method_like(every pair of six generic and momentum inputs, plus the non-vector error) comparenjitresults with pure Python on class, coordinate classes and values. All 24 fail onmainwithUntypedAttributeErrorand pass here.tests/backends/test_numba_object.py: 69 passed; full suite 4943 passed, 16 skipped (ROOT, CuPy, JAX, dask-awkward not installed); doctests 119 passed (Windows, Python 3.13, numba 0.68.0). Pre-commit hooks pass on the changed files; the local mypy hook environment reports[type-arg]errors only in untouched files.Checklist
$ pre-commit run --all-filesor$ nox -s lint)$ pytestor$ nox -s tests)$ cd docs; make clean; make htmlor$ nox -s docs) Not run locally; this PR does not change the docs.$ pytest --doctest-plus src/vector/or$ nox -s doctests)Before Merging