feat(api): proper LiquidHandlingPropertyByVolume class and validation for setting liquid class properties

[jbleon95]

Overview

Closes AUTH-837

This PR adds a proper LiquidHandlingPropertyByVolume class, replacing the previous dictionary of string keys to float values that was the literal representation of the liquid class schema. Now it uses that dictionary to make a default value as well as functions to take any volume and interpolate the proper volume based on that. Methods are also there for setting new points for volume and deleting existing points.

The other part of this PR is adding validation for the different settable values in liquid class that would require validating before hand. This includes boolean values, floating points, positive ints and floats, and xyz coordinates.

Changelog

• added LiquidHandlingPropertyByVolume class to handle interpolation for these properties and adding/deleting points
• added validators for all settable liquid class properties that need them for clear errors

Review requests

Should all the new validators live in the general PAPI validation.py class or should they exist elsewhere?

Risk assessment

Low.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.43%. Comparing base (094be6e) to head (91ce7c8).
Report is 81 commits behind head on edge.

Additional details and impacted files

@@           Coverage Diff           @@
##             edge   #16725   +/-   ##
=======================================
  Coverage   92.43%   92.43%           
=======================================
  Files          77       77           
  Lines        1283     1283           
=======================================
  Hits         1186     1186           
  Misses         97       97           

Flag	Coverage Δ
g-code-testing	92.43% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

[sanni-t]

Looks mostly good! Just a few suggestions. The only thing that sticks out to me is the properties_by_volume property.

[sanni-t]

What if we we make a separate function for fetching the sorted dict and add caching to it so that this function doesn't end up doing sorts again and again on the same set of data? Slight efficiency improvement

[ddcc4]

Can we just store the raw values sorted? Do we really need to preserve the original unsorted order that the user gave us? (But also, I'd expect the properties are so small -- like 5 entries -- that the sort time is negligible compared to everything else you're doing.)

[jbleon95]

I think I'll take a combination of yours and David's suggestion and just store these values in the class and update them whenever a value is added/deleted. It's functionally identical to caching it since that value would need to be updated anyway anytime a value is added/removed.

[sanni-t]

Also, not sure if the interpolator goes through the trouble of actually doing the interpolation even for values that are readily available in the dictionary. If it does, then I think it'll be a bit better to first check if the volume exists in the dictionary and interpolate only if it doesn't exist.

[sanni-t]

Hmm.. I wonder if there's a more intuitive way of representing this. Ideally would be good to avoid a call like aspirate.flow_rate_by_volume.properties_by_volume.
I guess maybe calling this property/ function something like get_dict() might also be an option

[ddcc4]

How about calling this something like raw_properties(), to distinguish it from get_for_volume() below?

[jbleon95]

I think I'll go with as_dict for now, can always update/change it until release.

[ddcc4]

How about calling this something like raw_properties(), to distinguish it from get_for_volume() below?

[ddcc4]

Can we just store the raw values sorted? Do we really need to preserve the original unsorted order that the user gave us? (But also, I'd expect the properties are so small -- like 5 entries -- that the sort time is negligible compared to everything else you're doing.)

[ddcc4]

How does the default play into this function?

Also, for someone not so familiar with numpy, and not so familiar with the expected behavior of the machines, it'd be helpful to have a comment that just outright states that you're trying to do a piecewise linear interpolation.

[ddcc4]

Hm, I'm still confused about the behavior of default. Let's say someone gives you this config without any non-default points:

{"default": 5}

Is get_for_volume() expected to return 5?

[ddcc4]

FYI, forcing the control points on the interpolation curve to be strictly positive makes life annoying for users. Like, if I want to configure a rate of 3 per uL, the easiest way to do that would be:

{
0 uL: 0.0,
1000 uL: 3000.0 
}

But if you reject 0 uL because it's not a positive float, then it's harder to set up the curve.

[jbleon95]

0 is an allowed value for this, I can rename this validation function if need be since I know zero isn't a positive number.

[ddcc4]

I can rename this validation function if need be

I would prefer that, but otherwise this PR looks fine. I can't think of a good short name for "ensure positive or zero" though.

I will note that in the future, there will probably be things that we need to be strictly positive (flow rates, dimensions) and some things that can be positive-or-zero (like the control points for interpolation). If you claim the ensure_positive_float() name now, someone who wants to implement "ensure strictly positive float" later will have to work around your naming.

[ddcc4]

Hm, I'm still confused about the behavior of default. Let's say someone gives you this config without any non-default points:

{"default": 5}

Is get_for_volume() expected to return 5?

[sanni-t]

Changes look good! Thanks!