WIP: Make Quantity an abstract base class
#822
Conversation
github-actions
bot
added
part:docs
github-actions
bot
removed
part:docs
llucax
force-pushed
the
quantity-abc
branch
2 times, most recently
from
1d66c40 to
2ebed70
Compare
github-actions
bot
added
part:docs
Otherwise unhandled exceptions will be silently swallowed by the task, which makes debugging really hard. Signed-off-by: Leandro Lucarella <[email protected]>
Signed-off-by: Leandro Lucarella <[email protected]>
We are going to make `Quantity` a abstract base class, so we don't need to do the trick to only allow to call the default constructor for the `Quantity` class but not sub-classes. We just make `Quantity.__init__()` always raise a `TypeError`, as subclasses can use `_new()` to create instances. Signed-off-by: Leandro Lucarella <[email protected]>
We don't want `Quantity` to be instantiated by using `zero()` or `from_string()` either. Signed-off-by: Leandro Lucarella <[email protected]>
This was possible before too, but using the `base_value` property, which is removed by this commit. Now the more idiomatic `float(quantity)` conversion is provided, although it should only be used for generics, when functions or classes are parametrized with a `SupportsFloat` type. `float(quantity)` always returns the quantity in the `base_unit`. This allows to make generic classes and functions that can operate on both `Quantity` or raw `float`s (or any other type that supports float). The methods `isnan()` and `isinf()` are removed because now `math.isnan()` and `math.isinf()` (as well as other `math` utility functions) can be directly used with `Quantity`. Signed-off-by: Leandro Lucarella <[email protected]>
Now that `Quantity` is an abstract base class, we can't really instantiate the base class directly in tests, so we need to make some adjustements and test that `Quantity` can't be constructed in any way. Signed-off-by: Leandro Lucarella <[email protected]>
Now classes and functions will be parametrized in a way that can take any type that can be converted to `float`, including `float` itself. Signed-off-by: Leandro Lucarella <[email protected]>
Instead of using the old `QuantityT`, we use `SupportsFloatT`, which allows us to use both quantities and raw floats for samples. Signed-off-by: Leandro Lucarella <[email protected]>
Instead of using the `QuantityT` type variable, we use the `SupportsFloatT` which allows for more flexibility in the type of samples that can be stored in the ring buffer, for example, supporting raw `float` values. Signed-off-by: Leandro Lucarella <[email protected]>
The resampler can now be paremetrized on both quantities and `float`. Even more, the values a resampler resamples are now attached to the specific type, not to the base `Quantity` class, so users can be sure the resampler will always return the appropriate quantity. If a resampler must be used to resample different types of quantities, it can now be parametrized using plain `float`s, which should also yield better performance as it avoids creating `Quantity` objects. Signed-off-by: Leandro Lucarella <[email protected]>
The data sourcing actor will now use `Sample[float]` to send samples instead of a unit-less `Quantity` base class that provides no value and decreases performance. Signed-off-by: Leandro Lucarella <[email protected]>
Same as with the data sourcing actor, the resampling actor will now use `Sample[float]` to send samples instead of a unit-less `Quantity` base class that provides no value and decreases performance. Signed-off-by: Leandro Lucarella <[email protected]>
This is just a subclass of `ResamplerConfig` that is specialized with `float`. Since the `ResamplerConfig` now needs more configuration (the type and the value constructor), and the `ResamplerConfig` is passed around quite a bit during initialization, it makes sense to have a class that already provides the basic stuff for `float`, which is what will always be used by the resampling actor because it needs to deal with different types of quantities (and is also good for performance reasons). Signed-off-by: Leandro Lucarella <[email protected]>
Because the resampler is now generic, we can also make the `MovingWindow` and `PeriodicFeatureExtractor` generic so they can return a specialized quantity instead of a unit-less quantity, again improving performance and safety. Signed-off-by: Leandro Lucarella <[email protected]>
|
When fixing the tests I bumped into an issue that might be tricky to solve. If I understand correctly, all the So far it seems like there is a lot of code and complexity to deal with As it is taking me a quite a bit of time trying to understand all the formula engine stuff, in particular when thinking about input and output types, I will delay the work on this PR. It is not very clear to me which stuff should work with We need to make sure to use the To progress with this PR I will probably add a second parameter for formula engine stuff that could have a different input and output type, so we have something like |
|
I thought that maybe to reduce the complexity of this formula engine stuff we could pin the type to The problem is formula engines are really "leaked" to the user, so users can do I guess this is also a shame in terms of performance as we might have a lot of conversion going on inside the pipeline that is not strictly necessary. Not sure if there is a way to have all the internal formula engine stuff working with floats and only convert to quantity when leaving the engine, I guess we would have to put (a lot) more thought into it to find out. |
|
FYI, I cleaned up this PR a bit, it is still a WIP, but except from the commits with |
|
This might have a new chance after this PR is merged: |
This is a draft PR just to make sure there are no big objections to moving forward with the idea.
The draft is there mostly to show how the new quantity and
SupportFloatTis used, the_quantitiesmodule in particular has a lot of noise, so it shouldn't be reviewed in detail.The code in this PR passes
mypyandflake8checks onsrc, but not intestsorbenchmarks, as these are not updated yet.The most important things to note are:
It uses
SupportFloatTas aTypeVarbound toSupportsFloatto allowSampleand most other types working with eitherfloatorQuantitiesto be used.This means for the generic types needing to actually use the underlaying value, we sometimes need an extra conversion to
floatwhen some operation is needed. This only happens inSample3Phaseto get themax/min, and the resampling function. It happens also implicitly when checking for NaN of infinity.Added a value constructor to the resampler to be able to produce new samples. This has the nice side effect that if you need to resample something that you know is in, say, kW instead of W, you can use
Power.from_kilowattsas the constructor.There are quite a few (unfinished) fixes/improvements to Quantity and its sub types. The most important changes to
Quantityare:__init__and the utility_newmethod is added to create instances..base_valuewas replaced by__float__, as this was only exposed to be able to work with aQuantityas if it was an unknownfloat.QuantityTis not needed anymore and removed.Quantityis now an ABC._base_valueis moved to the constructor, to have better rendering for the docs.isnan()andisinf()as now themathfunctions can be used directly instead (maybe we can keep them for convenience).matchinstead ofif, which needs to be cleaned up.Of course once this have a rough approval, I'll split in smaller commits (and even PRs) and update tests and docs.
Fixes #821.