HPInc / HP-Digital-Microfluidics

HP Digital Microfluidics Software Platform and Libraries
MIT License
2 stars 0 forks source link

Clean up uses of `ZERO` #212

Open EvanKirshenbaum opened 5 months ago

EvanKirshenbaum commented 5 months ago

All of the Quantity relations now take their arguments as ZeroOr[D] values, defined as

ZeroOr = Union[Literal[0], D] 

I'd like to take a pass through the MPAM code to get rid of most (or, if possible, all) of the uses of, e.g., Volume.ZERO and just replace them by 0.

This will be easy in things like

if volume > Volume.ZERO: ...

It will be slightly trickier in things like

def communicate(self, cb: Callable[[], Optional[Callback]], delta: Time=Time.ZERO) -> None: ...

as it will require changing the declaration of delta to be ZeroOr[Time]. And, thinking about it, I'm not sure that I really want to encourage passing in a bare zero as an argument if it's not completely clear that there's one dimension. (But maybe I do. I'll have to think about it.) In any case, it won't be possible for parameters in which the argument is something like DelayType, where there are two possibilities (Time and Ticks) that could both be meant, and the distinction is important.

Another case that probably will be worth doing is converting things like

delta = max(Ticks.ZERO, tick-self.next_tick)

to something like

delta = Ticks.max(0, tick-self.next_tick)

or possibly

delta = (tick-self.next_tick).max_with(0)

or

delta = (tick-self.next_tick).clipped(0, None)

Looking at the code in core.py, I also see that things like __add__() don't allow bare zeroes. It seems likely that anyplace where I'm calling _ensure_dim_match() should probably be taking ZeroOr[D]. (Also, _ensure_dim_match() itself should take D, not Quantity.)

Migrated from internal repository. Originally created by @EvanKirshenbaum on Oct 26, 2022 at 3:19 PM PDT.
EvanKirshenbaum commented 5 months ago

This issue was referenced by the following commits before migration:

EvanKirshenbaum commented 5 months ago

Some, but not all, of this is done. Specifically:

What I haven't done is replace T.ZERO default argument values with 0. Most of the time this doesn't seem as though it would buy much and sometimes (as with DelayType arguments, you want to know whether you're talking about Time.ZERO or Ticks.ZERO.

The one place it looked as though it might be reasonable was on a parameter that was passed to T.noise() as the standard deviation, but there if I don't have a quantity, I can't actually create the returned value, which should be zero in that dimension. I could redefine noise() to return ZeroOr[D], which would actually work in the one place I use it, but I'm not sure that's a good idea (or at least worth it).

Migrated from internal repository. Originally created by @EvanKirshenbaum on Oct 06, 2023 at 4:40 PM PDT.
EvanKirshenbaum commented 5 months ago

While cleaning up the code base (#305), I changed things so that you can't create a Quantity directly (without cheating). You're pretty much restricted to multiplying by a UnitExpr or calling Scalar.from_float(), both of which go through Dimension.make_quantity().

A (desired) side-effect of this is that make_quantity() is defined as

    def make_quantity(self, mag: float) -> D:
        if mag == 0:
            return self.ZERO
        return self.quant_class(mag, self, _dc = _DirectCreation.INST)

which means that all zero values for the same dimensionality will be the same object.

_DirectCreation.INST is a private tag object that is required to be passed to Quantity.__init__() and its overrides to ensure that people don't try to call, e.g., Volume as a constructor.

Migrated from internal repository. Originally created by @EvanKirshenbaum on Oct 19, 2023 at 3:17 PM PDT.
EvanKirshenbaum commented 5 months ago

I realized that when I added _DirectCreation I forgot to change Money to use it. That's now fixed, and you now have to multiply by a Denomination to get a Money value (other than ZERO or INF). To support this, Dimensionality.make_quantity() now takes optional keywords, which it passes on to its quantity constructor:

    def make_quantity(self, mag: float, **kwds: Any) -> D:
        if mag == 0:
            return self.ZERO
        return self.quant_class(mag, self, _dc = _DirectCreation.INST, **kwds)

This allows an optional CurrencyProxy to be passed in by Money.same_dim() and Currency.__init__().

Migrated from internal repository. Originally created by @EvanKirshenbaum on Oct 28, 2023 at 11:29 AM PDT.