Skip to content

Design proposal: Base type (WIP) #371

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

Closed
angularsen opened this issue Jan 13, 2018 · 14 comments
Closed

Design proposal: Base type (WIP) #371

angularsen opened this issue Jan 13, 2018 · 14 comments

Comments

@angularsen
Copy link
Owner

angularsen commented Jan 13, 2018

NOTE: This is work in progress and early thoughts, but input is most welcome.

An interface or abstract type implemented by all quantity types, such as Length and Mass.

Motivation

  • Represent generic quantities in code, where we don't need to know if it is Length or Mass but want to communicate it is a quantity - we need to use object today
  • Get textual representation of quantity with someQuantity.ToString() => "5 kg", this works today except we need to treat it as object and we don't have access to overloads for things like culture and significant digits after radix
  • Enumerate units for this quantity as this currently requires reflection, a usecase is to dynamically show GUI for converting a passed in quantity (1 meter) to any of the other length units
  • Present generic quantities visually in a 2D graph by showing unit selector for the quantities to change unit and getting the values for each quantity in the selected unit, without hardcoding support for specific quantities

Discussion points

  • Should we change quantities from struct to class? As discussed in the double/decimal issue, with class we might benefit from supporting both double and decimal internal representations without doubling the code. With an interface, it will quickly be boxed anyway unless using generic methods instead
  • Should the new members be added to quantity base type or be added to the UnitSystem type. One thing to consider is what culture to use for things like abbreviations and number formatting, where UnitSystem instances are created explicitly with a given culture (default uses CurrentUICulture in Windows).

Proposal

  • Add interface IQuantity since structs can't inherit base types, but users should be wary that casting to interface means boxing the value with those performance implications
  • ToString() should reflect the original value and unit instead of using the base unit, which may be very different than the units the user is working with (addressed in Preserve value and unit #389)
  • Add getter property for QuantityType since quantities already expose this value
  • Add getter properties for original value and unit to present or filter on these values separately
  • Add getter property for unit abbreviations as current you must figure out to use UnitSystem.Default.GetAllAbbreviations()
  • Add getter properties for singular and plural names of quantity for both "ForcePerLength" and "Force per length" forms, to make it easier to textually describe the quantity type. Currently you use reflection to obtain QuantityType enum value and call ToString() on it to get "ForcePerLength".
  • Add methods to obtain singular and plural names of a given unit to get `"Newton
  • Add getter property for the quantity description in JSON so this can be displayed next to the quantity selector GUI
  • Add getter property for enumerating all units for this QuantityType as this currently requires reflection (see sample app https://github.com/angularsen/UnitsNet/pull/380/files#diff-6f7804b386e2c6cd78fb5a331e9c9a58R18)
    -UnitConverter.ConvertBy methods should not use reflection as this is unnecessary brittle and can be solved by generating code to check if from/to unit types are LengthUnit and passing that to Length.From() and Length.As() methods
  • Reduce code size by moving shared code into base type (or helper class), we currently have ~80 quantities. We already do use helper classes for heavy things like Parse() and ToString(), so not sure how much more to gain here.

Case studies

Converter app with hard coded quantities and using strings for conversions

https://github.com/angularsen/UnitsNet#example-creating-a-unit-converter-app

  • Since quantities are hard coded in the app, we can obtain the list of units like this:
if (selectedQuantity == "Length") { SetConversionUnits(Length.Units.Select(x => x.ToString())) } 
else if (selectedQuantity == "Mass") {} /* and so on */
  • Alternative is to use reflection to obtain list of units, but this is brittle
  • Use strings for quantities and units in the GUI lists and use UnitConverter to convert between two units of a quantity like this:
double centimeters = UnitConverter.ConvertByName(5, "Length", "Meter", "Centimeter"); // 500

Converter app with generic quantities #353

The main difference to the example above is that we want to obtain the list of units for any selected quantity, without knowing the quantity type.
We either have the quantity string ("Length" or "Mass"), or its equivalent QuantityType.Length and QuantityType.Mass enum values, but there is currently no intuitive way of getting abbreviations from this.

Challenge: Present the plural name of any unit

For listing units in the GUI. We can get "meter" and "newton per meter" from LengthUnit.Meter and ForcePerLengthUnit.NewtonPerMeter by calling ToString() and splitting the pascal case into lower case words, but it would only be singular and difficult to reliably present in plural form. The plural form is defined in JSON, so we can easily generate that.

Challenge: Present abbreviation of any unit

This currently requires you to know the quantity or unit type, in order to look it up statically via string x = Length.GetAbbreviation(LengthUnit unit) or dynamically via string x = UnitSystem.Default.GetDefaultAbbreviation(Type unitType, int unitValue)

This dotnetfiddle shows how to get all length abbreviations, given that we added LengthUnit values to the list and can resolve abbreviations based on that type:

object[] lengthUnits = Length.Units.Skip(1).Take(10).Cast<object>().ToArray(); // Assume we add these to the GUI list and read those back as `object`s at some point
Console.WriteLine("Length unit abbreviations: " + string.Join(", ", lengthUnits.Select(unit => UnitSystem.Default.GetDefaultAbbreviation(unit.GetType(), (int)unit))));

// Output:
// Length unit abbreviations: cm, dm, pica, pt, fathom, ft, in, km, m, µin

https://dotnetfiddle.net/9f5TiF

A base type could have the method `GetAbbreviation(object unit)

Discussion on converting from struct to class to support decimal #285

This is related, because we are here discussing whether to convert from struct to class in order to get the benefit of supporting both double and decimal in the same library without doubling the code size.

Fixes #354

Related issues

@angularsen angularsen changed the title Design proposal: Base type Design proposal: Base type (WIP) Jan 13, 2018
@0xferit
Copy link
Contributor

0xferit commented Jan 26, 2018

Great proposal!

Just one thing I couldn't understand. Why do we need QuantityType field?

Also, would it be better if we prepare a UML class diagram for this discussion? I think it creates less confusion than a text proposal.

@angularsen
Copy link
Owner Author

A QuantityType getter allows you to do things like this:

// Example 1: Filtering based on quantity type
var massQuantities = quantities.Select(q => q.QuantityType == QuantityType.Mass).Cast<Mass>().ToArray();

// Bad example though, because this is probably better anyway
var mass Quantities = quantities.OfType<Mass>().ToArray();

// Example 2: Condition based on quantity type
if (quantity.QuantityType == QuantityType.Mass)
{
    // Do something based on knowing this is a mass quantity, such as updating some GUI labels
}

// Again, I would often prefer the new C#7 pattern matching with 'is' operator
if (quantity is Mass)
{
    // We can now treat quantity as type Mass
    double kg = quantity.Kilograms; // Valid inside if-scope
}

So, I can't immediately think of any good scenario where QuantityType is very useful, but it just seems like one of these things that may be handy in some scenarios and costs nothing to add since our quantities already expose this property. I don't feel like removing it, but I also can't provide a good argument for keeping it :-)

angularsen added a commit that referenced this issue Mar 5, 2018
### Motivation
Preserve the original value and unit and add getter properties `Value` and `Unit` to expose them.
Use `Unit` in `ToString()` instead of the base unit representation, to better match the units the user is working with.

This is a significant rewrite of how values are stored and how conversions are performed in quantity types. It is also the first baby steps of introducing some common values in base types #371 .

**There should be no breaking changes in neither main lib nor JSON serialization.**

### Changes
Using example of `Length`, but this applies to all quantities:
- Change private field `_meters` to `_value` (still `double` or `decimal`, as before)
- Add private field `LengthUnit? _unit`
- Add `Value` getter prop (`double` or `decimal`)
- Add `LengthUnit Unit` getter prop, with fallback to base unit `LengthUnit.Meter` (1)
- Ctors without unit param assumes `BaseUnit`, backwards compatible
- Change `ToString()` to use `Unit` instead of `DefaultToStringUnit`, new test cases
- Mark `DefaultToStringUnit` as obsolete
- Add support for preserving `decimal` precision in `QuantityValue` (2)
- Defer conversion to base unit until calling properties like `.Centimeters` (3)
- Serialize `Value`+`Unit` instead of base unit and base unit value, backwards compatible, update test cases
- Make internals of UnitsNet visible to Json to reuse reflection abstraction code
- Rename variables in JSON serialization lib (use "quantity" term instead of "unit")

1) Default ctor (`struct`) and ctors without unit param assumes base unit, to be backwards compatible. We might want to change this in the future to require explicitly specifying the unit.
2) Needed to avoid precision issues going via double to construct decimal values.
3) Getting the value of unit properties is now twice as slow, as it first converts from `Unit` to base unit, then to target unit. Before this change, the first conversion was initially made when constructing the quantity. However, there are optimizations in-place to avoid converting if `Unit` already matches either base unit or the target unit.

### Gotchas
- Serialization output changed (different value and unit for same quantity)
- In #389 (comment) I discuss the choice to not go with breaking change with regards to unspecified unit, but why we may want to do so later to require explicitly specifying the unit
@rgomez90
Copy link
Contributor

What is the state of this? I saw in the v4 prerelease there is an IQuantity but it is not really implemented or I am using it wrong?

I am developing an desktop app for automatic measurements (Calibrations, etc.) and was trying to implement this library looking for sth. like MeasuringRange<T>, Transmitter<T> or Calibration<T> where T: IQuantity so I could work better with units, but I am having problems with the IQuantity interface.

@tmilnthorp
Copy link
Collaborator

@rgomez90, what issues are you seeing? IQuantity is in master, however there are only two properties on it currently. Unfortunately all other methods need a unit type enum that is specific to each quantity type.

@rgomez90
Copy link
Contributor

So if I have for example something like this

image

how should I do it?

@tmilnthorp
Copy link
Collaborator

Unfortunately you can't do something like this in C# at all. You can't impose operator constraints on generic types. The best we could do is prove Add/Subtract/Multiply/Divide methods on IQuantity that takes another IQuantity (and optionally the desired result unit, otherwise default to BaseUnit). A quantity mismatch would be a runtime exception in that case, not enforceable by compiler constraints.

@rgomez90
Copy link
Contributor

rgomez90 commented Oct 11, 2018

That would be nice.

Also to be able to create new Quantity like new Quantity<T>(double value, IUnit<T>unit) would be very useful in generic scenarios.
I saw sth. like that in CsUnits

I tried to implement this adding following

  • New interface IQuantity<T>
public interface IQuantity<T> : IQuantity where T : IQuantity<T>
    {
        T Add(IQuantity<T> quantity);
        T Substract(IQuantity<T> quantity);
        T Multiply(double scalar);
        T Divide(double scalar);
    }
  • Structs implement IQuantity<T>
  • Each unit struct implements IQuantity<T> methods like this (example of pressure):
public Pressure Add(IQuantity<Pressure> quantity)
{
    Pressure pressureStruct = (Pressure)quantity;
    return new Pressure(this.AsBaseUnit() + pressureStruct.AsBaseUnit(),Pressure.BaseUnit); 
}

public Pressure Substract(IQuantity<Pressure> quantity)
{
     Pressure pressureStruct = (Pressure)quantity;
     return new Pressure(this.AsBaseUnit() - pressureStruct.AsBaseUnit(), Pressure.BaseUnit);
 }

public Pressure Multiply(double scalar)
{
     return new Pressure(this.AsBaseUnit() * scalar, Pressure.BaseUnit);
}

public Pressure Divide(double scalar)
{
    if (scalar <= 0) throw new ArgumentException("Only positive values allowed", nameof(scalar));
    return new Pressure(this.AsBaseUnit() / scalar, Pressure.BaseUnit);
}

Could be this a possible implementation? But this way every time boxing will happen or I am wrong? I also run a few unit tests and all passed without errors. See I don't use a quantity when multiplying or dividing, since that would have a different "meaning" (and also forces units change).

Still wondering now, how to get the value through IQuantity... what does BaseDimensions mean?
For ex. for a pressure of 2 bar BaseDimension contains
image
what is this telling me?

I am starting to try to contribute to some projects (since I don't have lot of experience) and just want to give it a try here.

  • Last question: Why doesn't IQuantity inherit from IComparable and IComparable<T> so we have access to their comparing methods through it too?

@angularsen
Copy link
Owner Author

The implementation looks about right. Agree on using scalars for multiplication/division.
I can't think of a reason why IQuantity does not extend those interfaces so that sounds like a good idea too. Not too sure about the IUnit<TQuantity> idea though, could you please provide a usecase where that would be useful?

BaseDimensions are the SI unit dimensions of a quantity. All quantities in the world are derived from the 7 SI base units and we intend to use this to generate operator overloads in the future, since you can use it to determine that Length = Speed * Duration and similar.

I'll let @tmilnthorp chime in on your draft first, but I'm positive about a PR on this change.
Contributions are most welcome and it's awesome you want to help out!

Please target your changes on top of v4 branch so you don't run into unnecessary work merging it in later. Beyond that, looks good to me.

@tmilnthorp
Copy link
Collaborator

IQuantity will not work for WRC

@tmilnthorp
Copy link
Collaborator

FYI the reason you can't get the value from IQuantity is because they are different types for different quantities. Some are double, some are decimal. IQuantity would permit this, however WRC again would not work. Not sure how far we cant to leave it behind considering operator overloading already does not work.

@angularsen
Copy link
Owner Author

angularsen commented Oct 11, 2018

@tmilnthorp I am more and more convinced we need to generate a separate source code for WRC (Windows Runtime Component, for everyone else's reference), the same way I think it would be awesome to generate TypeScript/Flow/JavaScript using the same unit definitions and generator scripts as a base. It's just too much compatibility issues to keep track of and other languages will bring new capabilities or new constraints.

A digression, but...

To avoid the huge diffs for every PR, which is seriously becoming a problem, we should also consider these options:

  • Instead of having all generated code for all 80 quantities checked in, we can have only a representative set checked in such as Length (double), BitRate (decimal) and PowerRatio (logarithmic) to look at for PRs. You lose search and browse capability on github.com for all other quantities, but I don't think that is a big loss honestly. Build server will regenerate code for every build so it will work just fine.
  • Same for WRC and TypeScript etc, only keep a representative set of the generated code for N quantities so that for PRs that add a new unit, you get a diff not only for .NET, but also for WRC and TypeScript. If you modify .NET code generators, you only get .NET code diffs.
  • Crazy idea, but we could also move WRC and TypeScript etc to other repos, and use git submodules to to link them back in here. This way we can still run a single script to generate code for all of them, compile and run tests for all of them, commit N times and push N times from the same local script. Then you would have N pull requests to merge it all into their respective repos. I'm not too keen on this idea though.

@rgomez90
Copy link
Contributor

rgomez90 commented Oct 11, 2018

Regarding IQuantity I was thinking maybe a double Divide(IQuantity<T> quantity> would make sense to get the factor between two quantities (of the the same type).

For ex. in my app I need to calculate the nominal signal {ns} of a transmitter given an input {i}:

ns = 4 + ((i - RangeMin) / RangeDelta) * 16

with Divide(IQuantity<T> quantity> I could do

4.Milliamperes() +
input.Substract(Specimen.MeasuringRange.Minimum)
.Divide(Specimen.MeasuringRange.Delta) * 16.Milliamperes();

and return the nominal signal as for ex. ElectricCurrent.

A possible implementation could be

public double Divide(IQuantity<T> quantity)
{
     return this / (T) quantity;
}

I saw for ex. Temperature does not provide operator '/' (why?) but we could do:

public double Divide(IQuantity<Temperature> quantity)
{
     return this.AsBaseUnit() / ((Temperature) quantity).AsBaseUnit();
}

@angularsen
Copy link
Owner Author

angularsen commented Oct 11, 2018

I saw for ex. Temperature does not provide operator '/' (why?) but we could do:

Because 0 Kelvins != 0 Celsius.

So you construct Temperature T1 = 10 Celsius.
Then you divide by 2.
What unit should you divide by?
If you simply divide the value you now have 5 Celsius. If you divide in the base unit Kelvin, you get 283 Kelvin / 2 = -131 Celsius.

I believe we support double ratio = Temperature.FromDegreesCelsius(10) / TemperatureDelta.FromDegreesCelsius(2);.

So back to your proposal, yes, a numeric ratio makes sense for most quantities that share the same zero point, but Temperature is a good example of a quantity that has a linear relationship between the units/scales. A ratio number is no longer sufficient, you need an y = ax + b linear expression to represent it. To complicate it further, we have logarithmic quantities like PowerRatio where the ratio of two values in different units is no longer linear but logarithmic.

Long story short, I don't think we can provide division generically the way I currently understand it.

@angularsen
Copy link
Owner Author

I think largely this issue is solved by IQuantity and the upcoming #576 with a new Quantity type.
Closing this so we can instead create a new, more focused discussion later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants