MadsKirkFoged / EngineeringUnits

Working with units made easy with automatic unit-check and converting between units
MIT License
42 stars 10 forks source link

Ratios of units should return as a double #1

Closed wiwalsh closed 3 years ago

wiwalsh commented 3 years ago

An engineering example of this is the ratio of specific heats (Gamma). This should be a double. UnitsNet results in the correct output of the following

            var Cp = SpecificEntropy.FromKilojoulesPerKilogramKelvin(1.00);
            var CV = SpecificEntropy.FromKilojoulesPerKilogramKelvin(0.718);
            double gamma = Cp / Cv;

In EngineeringUnits this results in an implicit conversion error.

MadsKirkFoged commented 3 years ago

Im yet to find a good way of converting directly to double. But for now it can be done if you add a (double):

        var Cp = SpecificEntropy.FromKilojoulesPerKilogramKelvin(1.00);
        var CV = SpecificEntropy.FromKilojoulesPerKilogramKelvin(0.718);
        double gamma = (double)(Cp / CV);
wiwalsh commented 3 years ago

I've looked into this a little bit and my suggestion is to create a "unit" that is Dimensionless. You can then have an implicit operator that results in a double. It would be nice for other reasons to know when a calculation results in a dimensionless quantity. I haven't figured out how to implement this in EngineeringUnits yet though. Maybe you can point me in the right direction.

Side note: When I am debugging, results of math come up as UnknownUnit. Should these be cast to the desired output unit, is this a debugger issue, or something else?

wiwalsh commented 3 years ago

I have some code that maybe sort of almost works. I'm sure you can help point me in the right direction. If you add me as a contributor on this I can submit a draft pull request to get your feedback.

MadsKirkFoged commented 3 years ago

All mathematical operations between two units quantity returns in an UnknownUnit. We then 'later' run a check to see if it is the same unit as expected (gives an error if it is not) and converts it from UnknownUnit to the expected unit.

I would love to be able to have double conversion as an implicit operator instead of an explicit. I have spent some time trying to make this happen but it always results in an: Error CS0034 Operator '*' is ambiguous on operands of type 'BaseUnit' and 'UnknownUnit' EngineeringUnits

If you can find a way it would be great!

wiwalsh commented 3 years ago

Pull request has some attempt at addressing this feature/issue #3

MadsKirkFoged commented 3 years ago

I have had look at your pull request and at first I really like your idea with creating a baseunit as Dimensionless and hoped that idea could work. but it needs to go from UnknownUnit -> Dimensionless -> double in one line and Im not sure the compiler can do that

            MassFlow M1 = new MassFlow(1, MassFlowUnit.KilogramPerSecond);
            double test = M1 / M1;
MadsKirkFoged commented 3 years ago

Are we going down the wrong path here?

If we set public static implicit operator double(UnknownUnit Unit)

Then we have operators blowing up:

        public static UnknownUnit operator *(UnknownUnit left, double right) => left.baseUnit * right;
        public static UnknownUnit operator /(UnknownUnit left, double right) => left.baseUnit / right;
        public static UnknownUnit operator *(double left, UnknownUnit right) => left * right.baseUnit;
        public static UnknownUnit operator /(double left, UnknownUnit right) => left / right.baseUnit;

But what If we 'just' delete all operatorwith double?

and instead create something like this:

        public static implicit operator UnknownUnit(double Unit)
        {
            return new UnknownUnit(Unit);
        }

It might be able to work without any hardcoded double operator. You are welcome to test it out, else I will at some point

wiwalsh commented 3 years ago

There are other math things that come up as issues I haven't figured out yet either. As an example, with ideal gas calculations (as an example) you typically take a Log or Exp of a ratio. This would end up being a Dimensionless unit that would again require either implicit conversion to double or a new math function that takes Dimensionless as an input.

I'm not sure I've "solved" anything here as much as I've thrown wrenches into how EngineeringUnits works.
I still come back to the issue that I'm having converting from UnitsNet to EngineeringUnits is just that implicit conversion to double...
I'll keep thinking about it. I'm sure there is an eloquent solution.

MadsKirkFoged commented 3 years ago

I had a go with it again and maybe this actually works?

It passes all tests and all I throw at it

Below code all works now

            MassFlow M1 = new MassFlow(1, MassFlowUnit.KilogramPerSecond);
            MassFlow M2 = new MassFlow(4, MassFlowUnit.KilogramPerSecond);

            var test = M1 / M2;
            double test2 = M1 / M2;

I'll create more unit tests to see if it actually works all around. Will you try it out?

wiwalsh commented 3 years ago

I updated to the latest and this solves the original issue! Thanks for implementing it. Certainly cleaner than what I proposed. There may still be a use for Dimensionless, but it is not associated with this issue.

MadsKirkFoged commented 3 years ago

We have the unit Ratio which is the only dimensionless unit we have right now. But we could create an alias named Dimensionless if the name Dimensionless is being used places where the name Ratio cant be used. Thanks for opening this issue and the time you spend on it!