-
Notifications
You must be signed in to change notification settings - Fork 85
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
wip: add units implementation #2545
Conversation
I'm just playing around here. There are two separate possibilities for adding unit support:
This PR implements (2). I think (1) is a worse prospect, because it prevents users/library authors from adding array behaviors in addition to units, unless they re-implement the entire units mechanism themselves. Our current typestring override logic is not advanced enough to let users fully mimic a custom units typestring, so this would be noticeable. My current thinking is that we want to reserve behaviors for users/library authors, not core features; otherwise we preclude their use. That is to say, just because library authors could implement unit support using behaviors, does not mean to say that we should do so ourselves. @jpivarski how does that logic sound? |
I agree that (2) is a good way to go, and we can be opinionated about Pint registries being the preferred way to get units. I think we already know that Vector would want to "play well" (i.e. "compose") with the application of units, so we would either need to immediately make behavior-composition possible or develop built-in units with an eye toward compatibility with Vector. The latter is more pragmatic. |
@jpivarski I agree that behavior composition is less pragmatic — it is a can of worms given our existing 1:1 approach for names-classes. Does this mean that you're in favour of e.g. units being added to records, rather than their fields? I can see an argument in both directions and I'm on the fence; there's an elegance to nor limiting ourselves to You're right about |
I think units should be added to awkward/src/awkward/contents/content.py Line 105 in afd3855
Sorry if whatever I said led you to think about records. Vector defines behaviors on records, that's true. But if you define the right unit-behaviors on lib.sqrt(x**2 + y**2) where This is all that Vector is doing when it sets the fields of record arrays to
That's what I meant, too: that we can freely use Pint as though it's the only system for handling units because it's so well established. The "registry" I was referring to is I was not suggesting that we make the system of units pluggable the way that backends are pluggable. For something like this, we can ask people to express their units in Pint's terms. |
I was thinking about vector!
Thankfully, due to the above, it does work nicely with |
I don't see how we can use If we use backend = PintBackend(CupyBackend) But maybe we don't use In either of the two possible implementations, Vectors with units should pass through (i.e. compose), but that remains to be seen. An array of vectors could be constructed with different units in its |
That's what this PR does in three phases:
Yes, this! |
In #2545 (comment), "array" means a rectilinear buffer, not an That would be (e.g. for the |
No, it can wrap |
But, conceptually, an I'm not talking about technical difficulties in making a |
It might seem like I'm being pedantic on this, but I'm rather trying to elucidate the desired behavior. I think it does make sense to have units on records; it's a way of saying "all fields have the same unit". I also don't think we want to open that can of worms — thinking about it this morning, this would make operations of |
For several of the coordinate systems in Vector, the units on each field would be different.
In Vector, associations between the fields of the records are generally loose. For instance, they don't even need to be the same numeric type: (Can you tell I'm enjoying GitHub's |
I can now, haha! |
afd3855
to
67b0773
Compare
Codecov Report
Additional details and impacted files
|
pint
already partially supports Awkward Arrays:But this doesn't really scale to the nested, complex layout structures that Awkward supports.
This PR adds support for
__units__
parameters in arrays, which allows structures to contain mixed units. Closes #2468.ak.to_pint
andak.from_pint