Skip to content
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

Extending the functionality of BaseUnits & UnitSystem #651

Open
lipchev opened this issue Apr 15, 2019 · 8 comments
Open

Extending the functionality of BaseUnits & UnitSystem #651

lipchev opened this issue Apr 15, 2019 · 8 comments
Labels
enhancement pinned Issues that should not be auto-closed due to inactivity.

Comments

@lipchev
Copy link
Collaborator

lipchev commented Apr 15, 2019

Following the many lengthy exchanges concerning the BaseUnits (default) and the UnitSystem in #646 here is a summary of limitations that I think should be addressed at some point.
First let me start by stating the general use cases (the way that I see them- feel free to add/correct me here):

  1. Domain separation
  • Stock Screen works with quantities in kg | liters (UnitSystem)
  • Recipe Screen works with quantities in mg | ml
  • Persistence- use a well defined UnitSystem for storing values- forget about Quantity.To/FromXX- As(..), new (..) instead
  • Reporting- I want my report to use this and that measure unit (ideally UnitSystem would be serializable)
  1. Presentation customization
  • default unit, default abbreviation- ideally it is also possible to associate those with a range- [1e, 1e3] -> [Gram], [1e3,1e6] -> [Kilogram]
  • list of common units- what do you put in the combo-box

Currently, the default (abbreviation for) BaseType for a UnitSystem is determined by taking the first matching unit, that satisfies the equality comparison with the UnitSystem.BaseUnits. The following two operations depend on the success of the previous lookup:

  • Constructing quantities using the (double, UnitSystem) constructor
  • Converting using the As(UnitSystem) operator

This approach has the following limitations:

  1. Possible run-time exceptions due to some combination of quantity types in a UnitSystem not having a matching base type definition- it makes sense to throw an exception for custom UnitSystems- but at least constructing/converting using SI should be safe
  2. BaseUnits cannot be specified for prefixed units: [Kilo]Gram is not matched for SI as the BaseUnits apply to the Gram only
  3. Ambiguous BaseUnits definitions: using the "dimensions ignored" conversion may sometimes lead to ambiguous conversions such as with the Liter -> CubicMeter derivatives. Consider the following mapping:
  • 1 l = 1 dm3 -> Decimeter
  • 1 dl = 1 ? (0.1 dm3 | 100 cm3)
  • 1 ml = 1 cm3 -> Centimeter
  1. Overlapping BaseUnits- having the find-first approach makes it easy to both override/have your default unit definition overridden by mistake
  2. Mapping BaseUnits definitions for UnitSystems in run-time: there is currently no way to define missing or override existing BaseUnits for a UnitSystem

I think the last point is the key to solving most of the limitations on the list. My proposition would basically consist of storing a dictionary representing the default unit for a given quantity type inside the UnitSystem:

private readonly Dictionary<QuantityType, UnitInfo> _defaultUnits; // could be Lazy<..>

This dictionary could be constructed eagerly or lazily (as is currently the case)- using the existing convention (to start with):

        public UnitSystem(BaseUnits baseUnits)
        {
            ...
            // default implementation (does not fix the prefixed entities matching issue)
            _defaultUnits = Quantity.Infos.ToDictionary(x => x.QuantityType,
                                                        i => i.GetUnitInfosFor(baseUnits).FirstOrDefault());     
        }

The Constructors/As(UnitSystem) methods would no longer have to go about finding the "first matching base type" every time- instead they would get it directly from the UnitSystem:

    public double As(UnitSystem unitSystem)
    {
        .. 
        var defaultUnitInfo = unitSystem.GetDefaultUnitInfo(QuantityType);  // casting omitted for simplicity
        if(defaultUnitInfo == null)
            throw new ArgumentException("No default unit was defined for the given UnitSystem.", nameof(unitSystem));
        return As(firstUnitInfo.Value);
    }

There would of course be helper methods for the user to register his preferred defaults for a given quantity type like:

        public void RegisterDefaultUnit(QuantityType quantityType, UnitInfo unitInfo)
        {
            _defaultUnits[quantityType] = unitInfo;
        }

Some tweaking of the JSON schema/parsing would probably be required for solving the issue with the base types for the prefixed quantities.
// TODO discuss the usefulness of the points presented in the Presentation Customization section and/or see if any (/other) customizations might be of interest for a given UnitSystem

@angularsen
Copy link
Owner

Excellent, thanks!

@angularsen
Copy link
Owner

Just a quick 2 cent after my first read or your text.

Great write up, good starting point for discussions!

To add, these are the things I currently have the most questions about:

1. What are the usecases for UnitSystem? Why should anyone use it?

  1. Presentation (default units in textual representations of quantities)
  2. Consistent units for calculations (need more details here, is this really a point since Units.NET calculations already take both value and unit into account?)
  3. Persistence (one could reduce the output by serializing only numbers and the UnitSystem, but I think I would prefer value+unit pairs instead)

🤚 I would personally use it to easily switch between metric/SI units and imperial units in the presentation layer. I don't know of anything else I would use it for.

2. What other unit systems than SI can be represented? How?

  1. In Experimental: Add CGS and FPS unit systems #630 I tried adding CGS and FPS unit systems, but UnitSystem is currently designed to represent the 7 SI base units and there is no 1:1 mapping to other unit systems. I believe the same goes for English/British Engineering units.

🤚 I would personally only be interested in adding English Engineering units, I think.

@lipchev
Copy link
Collaborator Author

lipchev commented May 6, 2019

On your first set of remarks:

  1. Yes, presentation is one- value input is another point of interest: I imagine the input of the dimensions of a 3D object- there are at least 3 sets of {numeric-input : combo-box} controls(or embedded in a single control- doesn't matter), all of which should presumably have their "combo-boxes" point to the preferred user unit- that in some CAD software applications is defined when starting a new 'project/sheet etc'
  2. No, I don't think one would need UnitSystem for calculations- business logic deals with Quantities (unless there is some special use for it with 'magic' quantities- maybe some abstraction for mapping between UnitSystems?)
  3. I don't think of it that much in terms of storage, but rather in query performance: say I want to do a range search- I wouldn't want to perform the conversion in sql- that's way too many switch/cases.
    PS I haven't done the PoC- but I reckon it should be possible to get some IQueryable extensions using the expression builder magic- if anyone has done the work already- I'm interested in both the one and two column scenarios.

@lipchev
Copy link
Collaborator Author

lipchev commented May 6, 2019

On the second point about representing other 'actual unit systems':

I've had a look at BaseDimensions- and from what I can tell there is nothing that would break were it to become something like a dictionary of { Dimension(enum): Value(int) }. Alternatively, this could be a fixed size array. As such one could imagine it having different implementations for different UnitSystems. SI: defining the 7 dimensions, EE: defining 6 dimensions (5 in common with SI).
Defining those in JSON would require something similar to the implementation for the Localizations- giving each Quantity a dictionary of BaseDimensions- one for each defined UnitSystem.

Now, there is of course the question of converting to and form Dimensions that are not part of a given system- and note the term 'Dimension' that I've used- it refers to the first column here.
This term would have to replace the QuantityType parameter from the examples in my previous post. As such one would imagine an overload that goes something like:
var tempDegRankine= temperatureAs(UnitSystem.EE, Dimensions.AbsoluteTemperature)
Alternatively each UnitSystem should provide its own conversion functions to/from SI- similar to how Quantities are converted between Units in a QuantityType.

I guess it could work but seems to me like a bit of an overkill. At least if the end-game is simply to have a way of defining default units per QuantityType. What other use can you imagine for something like EE as opposed to myEngineeringUnits?

@stale
Copy link

stale bot commented Sep 24, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Sep 24, 2019
@angularsen angularsen added pinned Issues that should not be auto-closed due to inactivity. and removed wontfix labels Sep 24, 2019
@angularsen
Copy link
Owner

Bad bot. This is still an issue we need to address.

@lipchev
Copy link
Collaborator Author

lipchev commented Sep 24, 2019

If you want I could make a PR of the first bit here: allowing the in-code registration of Default Units, as described in code snippets from the first post. This should allow (for the time being) for issues like the one described in #700 to be patched in the client code.

I'm still on the fence on the actual use cases.. In my models(over the past few months) there seems always to be the case for two quantities(of same type) to have the need for different default units(e.g. the weight of the flask defaults to 'grams' while the substance in it: 'milligrams'). That would make for two unit systems- which might make sense some times but would require quite a bit of overhead if implemented as a DP. Same goes for the repositories- if I had to know to use the "FlaskUnitSystem" instead of the general (let's call it) "MyRepositoryUnitSystem"- then that kinda beats the purpose of using a unit system in the first place.

Not to mention that encouraging people to use something that might fundamentally change in design is also a little problematic.

@lipchev lipchev closed this as completed Sep 24, 2019
@lipchev lipchev reopened this Sep 24, 2019
@angularsen
Copy link
Owner

angularsen commented Sep 24, 2019

I'd be very interested in seeing a PR to try to move the discussion forward. There are a lot of ideas and thoughts here and other threads, so some concrete changes would be very helpful to discuss around.

I don't have a lot of time to spend on this project lately, so it's hard to keep track over long periods of time and having to keep refreshing my memory on how this all works :-)

You already described it well above, but I'm taking a stab at a design proposal here just to have something concrete to discuss and move forward faster.

Design proposal

Use cases

  1. Configure default unit per quantity at runtime for a unit system
  2. Configure default abbreviation per unit at runtime for a unit system
  3. Construct quantity given a number and unit system
  4. SI and English Engineering unit systems included out of the box with mappings for all quantities
  5. Create an immutable variation of a unit system like this: UnitSystem siVariation = UnitSystem.SI.WithDefaultUnit(QuantityType.Length, LengthUnit.Centimeter)

These, however, feel a bit more like application configuration to me:

  1. List of unit choices to display to user for a quantity in a unit system
  2. Configure unit ranges per unit system, f.ex: SI: [0,99) cm, [1,1000) m, kilometers etc.. and EE: [0,3) in, [1,1760) yd, miles etc.. I had this particular need for distance measurement in an application years ago.

It doesn't mean we shouldn't add support for it, but maybe it is a different concept than "unit system"? Different types/helpers? Or maybe it's fine.

Changes

  1. Remove BaseUnits type and remove related from JSON and UnitSystem
  2. UnitSystem: Add mapping of default unit, such as QuantityType.Length => LengthUnit.Centimeter
  3. UnitSystem: Add mapping of default abbreviation per unit enum value, such as LengthUnit.Feet => "ft"
  4. Length.ctor(double, UnitSystem) constructs with unit system's default unit for Length
  5. double val = myLength.As(UnitSystem) converts to unit system's default unit for Length
  6. UnitSystem should remain immutable, so to change its configuration at runtime you create a new instance either manually or by cloning an existing system with extension methods for adding/modifying mappings like in use case 5. Then you inject this instance into your view or whatever.
  7. Add UnitSystem.SI and UnitSystem.EnglishEngineering with mappings

Background / rambling

Ambiguous BaseUnits definitions: using the "dimensions ignored" conversion may sometimes lead to ambiguous conversions such as with the Liter -> CubicMeter derivatives

That hits the nail on the problem I think. I'm starting to realize we maybe approached this all backwards. Currently we define base units for all 1000+ units and try to look up the best match for a unit system, but this is often ambiguous and many times there are no meaningful mappings. Also, other standard unit systems (EE, CGS etc) don't map to the same 7 base units (dimensions) so we can't really represent them right now.

Instead, given the usecases above, it feels more natural that a unit system defines the default unit and abbreviation for one or more quantities so you can construct quantities and ToString() with consistent results.

I believe that solves the base unit ambiguity, allows us to configure prefix-units like kilogram and allows for any arbitrary unit system really as long as it defines the default units and abbreviations for the quantity you are trying to construct or call ToString() on. Maybe I'm forgetting something, but on the surface this feels like a more holistic solution to me, I am hopeful!

Please take a look at my proposal, revise it, and let's get a PR started 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement pinned Issues that should not be auto-closed due to inactivity.
Projects
None yet
Development

No branches or pull requests

2 participants