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

Allow Override of Default Unit Conversion #1313

Open
dustincleveland opened this issue Sep 5, 2023 · 3 comments
Open

Allow Override of Default Unit Conversion #1313

dustincleveland opened this issue Sep 5, 2023 · 3 comments
Labels
bug enhancement pinned Issues that should not be auto-closed due to inactivity.

Comments

@dustincleveland
Copy link

Our industry uses specific conversion ratios between units that are not always the standard. For example (according to my best understanding of what was explained to me), the conversion between pressure units of Pascals and Inches of Water Column is an approximation due to certain environmental factors. Therefore, we may want to adjust that conversion to slightly adjust that approximation.

I would like to be able to set a conversion function to override the default. I tried this:

// Using UnitsNET 5.32.0
using UnitsNet;
using UnitsNet.Units;

UnitsNetSetup.Default.UnitConverter.SetConversionFunction<Pressure>(
    PressureUnit.Pascal,
    PressureUnit.InchOfWaterColumn,
    pressure => new Pressure(999, PressureUnit.InchOfWaterColumn));

UnitsNetSetup.Default.UnitConverter.SetConversionFunction<Pressure>(
    PressureUnit.InchOfWaterColumn,
    PressureUnit.Pascal,
    pressure => new Pressure(999, PressureUnit.Pascal));

var pressure = Pressure.FromPascals(1);

Console.WriteLine(pressure.InchesOfWaterColumn);
Console.WriteLine(pressure.As(PressureUnit.InchOfWaterColumn));

// Expected Result: 999
// Actual Result: ~0.004

However, this does not seem to work because Pressure.TryToUnit is called before attempting to use the unit converter to retrieve the default conversion function. I'm not sure if that is a bug, or working as intended, as it is unclear to me how you would extend the PressureUnit enum such that Pressure.TryToUnit would fail to convert. Therefore, I also wonder how the unitConverter argument would ever be made use of? I feel like I must be missing something very obvious. Is it terrible for performance to check the unit converter for a conversion function before calling Pressure.TryToUnit?

It seems like the alternatives are to fork the code base or to define a custom Pressure unit. The latter option makes it unclear to developers which Pressure unit they should be using in the application, along with the API tradeoffs described in the documentation on custom units. Both options are pretty heavy for a slight tweak to the conversion factor.

@angularsen
Copy link
Owner

I'll need some time to get around to this.
It seems like a bug, that maybe custom conversion functions don't always have precedence over the built-in ones?

Not sure, but if you are able to debug to confirm this and write a unit test that fails, then passes with a fix, then that will help speed things up a lot.

@dustincleveland
Copy link
Author

Okay, I will do that!

@angularsen angularsen added bug pinned Issues that should not be auto-closed due to inactivity. labels Jul 8, 2024
@lipchev
Copy link
Collaborator

lipchev commented Nov 26, 2024

This is a limitation in the current implementation- there was a similar request back-when, and I remember we initially had the UnitConverter being the first one to get asked to do the conversion, but this had a significant impact on performance, so we switched it around- having the converter be called as a fallback to the TryConvertUnit, thus supporting only "the additional units". I guess it worked for somebody..

In retrospect even that was probably a mistake, as the (significant!) performance impact of boxing the Unit and calling in the concurrent dictionary and all that fun stuff is affecting all conversions between non-base units (e.g. Milligram-Microgram).

I've been toying with this (and the extensibility side as a whole) - here's what I got down in the sample project:

    public static void Configure()
    {
        UnitsNet.UnitsNetSetup.ConfigureDefaults(
            builder => builder
                // configure custom conversion coefficients while preserving the default QuantityInfo for the Pressure (which has the "Pascal" as the DefaultBaseUnit)
                .ConfigureQuantity(() => Pressure.PressureInfo.CreateDefault(ConfigureCustomPressureUnitConversions))
                // configure a "completely new" configuration for the TemperatureDelta
                .ConfigureQuantity<TemperatureDelta, TemperatureDeltaUnit>(CreateCustomTemperatureConfiguration)
        );
    }

    private static IEnumerable<IUnitDefinition<PressureUnit>> ConfigureCustomPressureUnitConversions(IEnumerable<UnitDefinition<PressureUnit>> unitDefinitions)
    {
        // we customize a subset of the existing unit definitions (preserving the rest)
        return unitDefinitions.Select(definition => definition.Value switch
        {
            // since these conversions don't involve a constant term we can only specify the conversionFromBase (with the inverse conversion assumed to be the inverse: e.g. 1/999)
            PressureUnit.InchOfWaterColumn => new UnitDefinition<PressureUnit>(definition.Value, definition.Name, definition.BaseUnits, conversionFromBase: 999),
            PressureUnit.MeterOfWaterColumn => definition.WithConversionFromBase(1234.5m),
            PressureUnit.MillimeterOfWaterColumn => definition.WithConversionFromBase(1.2345),
            PressureUnit.MillimeterOfMercury => new UnitDefinition<PressureUnit>(definition.Value, definition.Name, definition.BaseUnits, new QuantityValue(1, 3)),
            PressureUnit.Decapascal => new UnitDefinition<PressureUnit>(definition.Value, definition.Name, definition.PluralName, definition.BaseUnits,
                definition.ConversionFromBase.Coefficient * 1.2m,
                definition.ConversionToBase.Coefficient / 1.2m
                ),
            // all preceding conversions result in something of the form:
            PressureUnit.InchOfMercury => new UnitDefinition<PressureUnit>(PressureUnit.InchOfMercury, definition.Name, definition.PluralName, definition.BaseUnits,
                // f(x) = 1/3 * x
                new QuantityValue(1, 3),
                // f(x) = 3 * g(x)^1 + 0  // with g(x) => x in this case
                new ConversionExpression(
                    3,
                    null, // a delegate of the form QuantityValue -> QuantityValue (when null, g(x) = x)
                    1,
                    0)),
            // we keep all other unit conversions as they are
            _ => definition
        });
    }

    public static void OutputPressure()
    {
        var pressure = Pressure.FromPascals(100);
        Console.WriteLine(pressure); // outputs: "100 Pa"
        Console.WriteLine(pressure.As(PressureUnit.Kilopascal)); // outputs: "0.1"
        Console.WriteLine(pressure.As(PressureUnit.InchOfWaterColumn)); // outputs: "9990"
        Console.WriteLine(pressure.As(PressureUnit.MeterOfWaterColumn)); // outputs: "123450"
        Console.WriteLine(pressure.As(PressureUnit.MillimeterOfWaterColumn)); // outputs: "123.45"
        Console.WriteLine(pressure.As(PressureUnit.Decapascal)); // outputs: "12"
        Pressure pressureInInchesOfWaterColumn = pressure.ToUnit(PressureUnit.InchOfMercury);
        Console.WriteLine(pressureInInchesOfWaterColumn); // outputs: "33.33333333333333 inHg"
        Console.WriteLine(pressureInInchesOfWaterColumn.Value * 3 == 100); // outputs: "True"
        Pressure conversionBackToPascals = pressureInInchesOfWaterColumn
            .ToUnit(PressureUnit.Atmosphere)
            .ToUnit(PressureUnit.MeterOfWaterColumn)
            .ToUnit(PressureUnit.MillimeterOfWaterColumn)
            .ToUnit(PressureUnit.Decapascal)
            .ToUnit(PressureUnit.Pascal);
        Console.WriteLine(conversionBackToPascals); // outputs: "100 Pa"
        Console.WriteLine(conversionBackToPascals == pressure); // outputs: "True"
        Console.WriteLine(conversionBackToPascals == pressureInInchesOfWaterColumn); // outputs: "True"
    }

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

No branches or pull requests

3 participants