-
-
Notifications
You must be signed in to change notification settings - Fork 2k
Consider adding special "arbitrary" unit? #3793
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
Conversation
|
Since this is specific to how you wish to handle models, does it make more sense to make it a hidden internal unit within |
|
@pllim That's sort of what I'm thinking--wherever it gets defined I don't think it should be a normally user-visible entity. I'm wondering if this is a worthwhile approach at all though, or if I just need to rethink some things. I think it would be convenient though, as a way to encode this distinct behavior. |
|
Cc: @mhvk |
|
@embray , like you mentioned somewhere else, using this arbitrary unit instead of normal Numpy arrays will slow things down. Definitely something to think about. |
|
I need to think a bit more about this, but like it in principle as a more formal way than the current p.s Since this would be for quantity-fed models, it should not affect speed much I think (and Quantity will soon speed up, once |
@pllim I'm not sure what you're referencing. That's not the case. This is just needed in a few minimal contexts to wrap bare arrays with a "unit" indicating the behavior that the result will inherit units from any operands that do not have the "arbitrary" unit. What does slow things down is that in order to determine whether or not the "arbitrary" unit is in use means checking the values of each input. If an input is a large array that means we need to scan through the entire array. I'm not sure if there's a way around that though, except to disallow this behavior in models. I'm still split on whether or not to do that. |
|
@embray , perhaps I misunderstood your comment in the other discussion (can't find it now). I apologize. Regardless, I am also not sure which is the best approach. |
|
If it helps, I'll see if I can come up with a PR for this so we can see what impact it has. |
|
@embray - that would be great! I thought a little more about it and it seemed most logical to make a new unit class (perhaps subclassing The main thing would be to think about the operators. I think logically any multiplication/division with another unit returns |
|
p.s. I could also try an initial PR, though it would likely not be for a few days. |
|
That sounds more or less along the lines of what I was going to do, though I appreciate you typing it out since this gives a sort of rough checklist to follow. |
|
@embray - inspired by your work on getting Quantity support in modeling, I made an attempt at defining an arbitrary unit. Would this be what you are looking for? @mdboom - please have a look too, especially at (1) whether the way I ensure there is only one such unit makes sense; and (2) whether the implementation in From the added documentation: |
|
How about making it a little "hidden" like this? After all, this makes no sense for most u._arbitrary |
|
Thanks for this--this looks mostly like what I had in mind. The following example is confusing to me though: >>> u.m / u.s * u.arbitrary
Unit("arbitrary")I would have expected >>> u.m / u.s * u.arbitrary
Unit("m/s")But maybe there's a good reason it has to be like your example? |
|
To clarify my last example, in terms of Quantities I would expect something like I also agree with @pllim that we should make this for internal use only at the moment, though I have no problem with mentioning it in the changelog. |
astropy/units/core.py
Outdated
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.
Were these print statements supposed to stay in here?
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.
Darn, no...
|
@embray - I think it has to be as I have it: if the dimension of anything is arbitrary, then multiplying/dividing it with another would leave it arbitrary. For For multiplication, I think one does want what I wrote, because I would expect the following: Here, of course, I'm really thinking that the |
|
@pllim - I thought about making it hidden, but ended up feeling is was better to just have it documented. @embray mostly wants it for quantities with all 0, inf, nan, but in principle one could use it more generally. While I think this would mostly just throw away all the benefits of units, I didn't see a clear reason to preclude it. |
astropy/units/tests/test_units.py
Outdated
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.
arbirary -> arbitrary
I'm not sure about that. Currently, >>> 0.0 * (1.0 * u.kg)
<Quantity 0.0 kg>In other words, the value is zero but it takes on the units of whatever Quantity zero was multiplied with. Then I think that adding "zero kilograms" to any number of meters should be an error, and it is: >>> (0.0 * (1.0 * u.kg)) + (1.0 * u.m)
UnitConversionError: Can only apply 'add' function to quantities with compatible dimensionsI think only zero in "arbitrary" units should add with 1 meter. But when zero is multiplied with a quantity the result is not in arbitrary units, but in the units of whatever zero was multiplied with (as though it were dimensionless). |
|
In other words, what I expected is already the current functionality, which I think is correct. The only thing adding this "arbitrary" unit changes is it makes it explicit when a value being passed around will have this behavior, so that I don't have to repeatedly check if a value is all 0/nan/inf. |
|
@embray - the difference in your example is that you are, effectively, doing This is not the same as doing something with arbitrary units. Of course, the multiplication issue is also not the one we're trying to solve -- it works already fine without taking any special care of |
|
There's no difference between and with respect to multiplication. That's how things already work. What makes the "arbitrary" unit different from dimensionless is, as you say, its behavior with respect to addition. But |
|
Let me reiterate: To me the only purpose of this arbitrary unit would be to encode the existing functionality. You're proposing something different and, IMO, broken. Update: Perhaps to drive that point home we would even disallow creation of Quantities with "arbitrary" as the unit unless |
|
@embray - Further back to basics, I think it is obvious that But mostly I guess I don't quite see why we should generally enforce that something with arbitrary units can only hold 0, inf, or nan; that is not the usual definition at all. I do of course see that you want it in |
|
The problem is that the way it's treated, zero does not have default units of dimensionless_unscaled, because that's not how it behaves in addition. It does behave the same in multiplication. But because of the different behavior in addition it is a special case. I'm saying that |
|
As for your point about commutativity (actually associativity) I don't think it applies, because "arbitrary" is not a real unit and will not be used in such expressions. Or in other words, the algebra of units does not include an "arbitrary" value, so units including "arbitrary" does not play by the same rules. The only reason I proposed for it to exist is so that if I have a value that is meant to be treated as a Quantity I can always treat it as a Quantity, and not have to do things like |
|
OK, let's for the moment ignore the fact that there may be use for a general arbitrary unit (which I think there is) and focus on what you actually need! I think you want to do One option would be to have no But an easy alternative would be to use My logic had been a bit different: I assumed that one would need this behaviour only in situations where one knew that addition is going to be done, not multiplication, and that one would then explicitly use |
Yes. The only reason I referred to it as a "unit" is that it would go into the I appreciate your effort to try to make this work more broadly but I don't think that really makes any sense in self-consistent system of units. And yes, I like the idea that |
Ah, I see how that led to confusion. No, my use case was quite the opposite--for the purposes of modeling the point is that the code has no idea what operations are going to be done on these values. I just wanted that the operations on an input of |
|
@embray - OK, good to have it clarified. I think your use case requires quite a different PR than this one, and I'll think about how to implement a version of p.s. I'm not in favour of doing this by default -- really, this was a hack that makes a few numpy functions work which do comparisons with 0 internally, and there have been annoyed reports as is that |
|
I don't know that what it requires is completely different--most of this looks like what I had in mind, although there are probably some details I haven't considered. Anyways, I really appreciate you looking into this! |
|
This PR is pretty old, there are conflicts, and at the end of the discussions suggested this is not the desired approach. So, closing. |
Something came up in working on models with units, that I thought might be useful. Though if it's deemed too "weird" I could live without it (or perhaps this could be implemented just as a hack to use within the modeling package).
There are a couple of special cases in quantity arithmetic--namely the scalars
0.0andinf, in that they may take "arbitrary" units (see here). This allows expressions like the following:to be clear, this is only allowed at all with zero.
q + 1will raise an exception since you cannot add a quantity in meters to a dimensionless quantity (by default scalars get units of dimensionless_unscaled, except in the special case of zero and inf). In fact,q + 0is different fromq + (0 * u.dimensionless_unscaled). The latter is making it explicit that the zero we're adding must be a dimensionless quantity, and can't be added to meters. This is a good design choice.However, it makes things a little inconvenient in how I'm trying to implement quantities in models. What I would like to be able to do is when quantities are being used to evaluate a model, it is convenient to automatically convert all inputs to the model to
Quantityobjects (so that everything has a.unit). In the spirit of how mostQuantityarithmetic works, inputs without explicit quantities are automatically given units ofdimensionless_unscaled. However, I can't do this if the input is zero (or inf) because that actually gives different results than if the model were evaluated with the scalar0.0, with its ability to take arbitrary units.Maybe that's just something that should be rethought outright, though I think it would be confusing if models have different evaluation behavior from other ufuncs with respect to how they treat 0 when combining it with dimensionful quantities.
So I think it might be convenient to be able to create a Quantity whose value is either 0 or inf, and has a special unit called "arbitrary" that just implies the existing behavior of the bare 0/inf, as distinct from dimensionless_unscaled. This would simplify a lot of code so I don't have to keep treating them as special cases.