-
Notifications
You must be signed in to change notification settings - Fork 32
feat: add PyTree support for vector objects #637
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
base: main
Are you sure you want to change the base?
Conversation
That's cool @nsmith-! This is a nice application for pytrees where you want to make use of the vector objects for physics, but are restricted by solvers that only work with floats. We definitely should add this for NumPy as well - it should be rather straightforward as they are automatically considered leaf types. Awkward Arrays are much more complicated, I'm not sure if that should be supported for them, and if so how they should be decomposed. |
Requires #622. |
Is that the case though? From stuff = (np.ones(4), np.ones(5))
pytree.flatten(stuff) it looks like not
|
This is considering arrays as leafs. We're storing things as SoA in vector right? So flattening a vector obj with numpy array as leafs should work like in this example. |
Ok so it is making a distinction between |
Well, it's a bit messy, they can be either: vec_soa = vector.obj(x=1, y=1) * np.ones(10)
print(pytree.flatten(vec_soa))
vec_aos = vector.array(dict(x=np.ones(10), y=np.ones(10)))
print(pytree.flatten(vec_aos)) gives
Note |
I'm not sure what the existing ravel in optree is missing that we want to add here? Shouldn't that be enough once the tree decomposition works? |
Ok, I see. Looks like the |
Yes, so from optree.integrations.numpy import ravel_pytree
ravel_pytree(vec_soa, namespace="vector") works, but I'd rather have |
For AoS arrays, we may have to ask optree to implement something, as from optree.integrations.numpy import ravel_pytree
data = np.array(
[
(1.0, 2.0),
(3.0, 4.0),
(5.0, 6.0),
],
dtype=[("x", "<f8"), ("y", "<f8")],
)
flat, unravel_func = ravel_pytree(data) returns
which isn't friendly to the scipy methods: from scipy.optimize import minimize
# works
minimize(lambda d: np.hypot(d[::2], d[1::2]).sum(), data.view(np.float64))
# does not work
minimize(lambda d: np.hypot(d['x'], d['y']).sum(), data) |
field_dtypes = [dt for dt, *_ in v.dtype.fields.values()] | ||
target_dtype = field_dtypes[0] | ||
if not all(fd == target_dtype for fd in field_dtypes): | ||
raise ValueError("All fields must have the same dtype to flatten") | ||
array = numpy.array(v).view(target_dtype) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We manually flatten the numpy struct dtype here and include the necessary information to revert it. For safety we require all fields to have the same dtype. This is necessary to fully flatten the array to a 1d vector
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TIL: optree
This is quite an amazing addition. Thanks for working on this, @nsmith-! I am a bit on fence with the API design.
The register_awkward
functions in Vector returns nothing and lets users pass Vector behavior names to the backend library (awkward
) directly (vector.register_awkward(); ak.Array(..., with_name="Momentum3D")
).
It would be nice to have an identical API for pytrees. From my very short experiments, an awkward
-like API for optree
looks feasible -
def _flatten2D(v: Vector2D) -> tuple[Children, MetaData]:
children = v.azimuthal.elements
metadata = type(v), type(v.azimuthal)
return children, metadata
def _unflatten2D(metadata: MetaData, children: Children) -> Vector2D:
backend, azimuthal = metadata
return backend(azimuthal=azimuthal(*children))
optree.register_pytree_node(
VectorObject2D,
flatten_func=_flatten2D,
unflatten_func=_unflatten2D,
namespace="vector.pytree"
)
and then the user can -
leaves, treedef = optree.tree_flatten(vec, namespace="vector.pytree")
akin to how the awkward
backend works (ak.Array(..., with_name="Momentum3D")
).
We can then have a vector specific constructor (like vector.Array
and vector.zip
), which I think in this case is the return value of vector.register_pytree
.
Could you see if this is feasible? Looking at how the awkward
backend is structured will be very useful. Thanks again for working on this!
The import optree
leaves, treedef = optree.tree_flatten(vec, namespace="vector") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @nsmith-! Could you add that way of using optree for Vector classes in the example? I have a few other structural suggestions for the docs:
- The example should be a notebook, so that it can be tested on our CI. The notebook is definitely a tutorial so it should be moved under
Tutorials
. Can the API documentation be moved somewhere else? Maybe a new page underVector Constructors
? - Vector backends in README should be updated (perhaps with a new heading below backends?)
- The documentation tree in README should be updated
- Same updates for docs/index.md
pre-commit is spellchecking the base64-encoded png image 🙄
edit: potentially https://stackoverflow.com/questions/78867158/how-to-make-codespell-not-report-false-positives-in-base64-strings is a better solution |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the updates, @nsmith-! This looks good to me now 🚀
@henryiii @pfackeldey please feel free to merge once you approve.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
If anything I'd suggest to add some explanation in the docs that this feature does not work with the awkward-array backend, that may not be obvious on first sight when someone sees this feature. You can decide, I have no strong opinion on this.
Description
See https://blog.scientific-python.org/pytrees/ for the rationale for these functions.
As a demonstration, here's a projectile motion with air resistance simulation using vector and scipy, showing in particular the use of
vector.pytree.flatten
to wrap the SciPy ODE solver:producing

Checklist
$ pre-commit run --all-files
or$ nox -s lint
)$ pytest
or$ nox -s tests
)$ cd docs; make clean; make html
or$ nox -s docs
)$ pytest --doctest-plus src/vector/
or$ nox -s doctests
)Before Merging