MadsKirkFoged / EngineeringUnits

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

Added SI Getter to SpecificThermalResistance #10

Closed ikijano closed 2 years ago

ikijano commented 2 years ago

Fixes SpecificThermalResistance.SI getter

MadsKirkFoged commented 2 years ago

Thanks for the contribution!

The SI getter was missing and you have add that correct!

Im at little unsure what you try to show in the first unittest:

            double P = 10.0;
            double L = 2.0;
            double T = 4.0;

            var expected = new SpecificThermalResistance(Length.FromMeters(L) * Temperature.FromKelvins(T) / Power.FromWatts(P));
            var expected_SI = (double)expected.SI;

            var actual = new SpecificThermalResistance(Power.FromWatts(P) / Length.FromMeters(L) * Temperature.FromKelvins(T));
            var actual_SI = (double)actual.SI;

            Assert.AreEqual(expected, actual);
            Assert.AreEqual(expected_SI, actual_SI, 1e-9);

Was this what you are trying to?

            double P = 10.0;
            double L = 2.0;
            double T = 4.0;

            SpecificThermalResistance expected = (Length.FromMeters(L) * Temperature.FromKelvins(T)) / Power.FromWatts(P);
            ThermalConductivity actual = Power.FromWatts(P) / (Length.FromMeters(L) * Temperature.FromKelvins(T));

            Debug.Print($"{expected}");

            Debug.Print($"{1/actual}");

            //Assert.AreEqual(expected.SI, (1/actual).SI, "some tolerance");

The last unittest can't compile - Maybe you can give that one, one more try?

ikijano commented 2 years ago

I'm not sure what I was thinking. That test doesn't make any sense and it seems that some reason test didn't even run so I have thought everything is ok. Sorry about that.

I rewrite tests and give another try.

MadsKirkFoged commented 2 years ago

Looks good!

ikijano commented 2 years ago

That was my first contribution ever! Hopefully it helps others also. Tests are far from perfect, but i tried cover at least basics.

I found there is possibility cause null pointer exception in constructor in general but to cover that subject is another topic and needs a lot of more unit tests to cover it.

I found this library about two months ago and found it very useful but it was lacking this feature what I really needed in my project.

MadsKirkFoged commented 2 years ago

I am happy that you make a contribution! If you have other feature requests or bugs, feel free to post them! Also if you want feedback on a pieces of code where you use this library, you are also welcome to post it! That way you are also helping other by showing how they can use this library.

ikijano commented 2 years ago

I found this library could be handy after I had played little bit with SharpFluids and latter heard recommendation from some random YouTube channel. Sadly I cannot directly show code how exactly I use this library and it's not actually essential in that project but it makes code more readable, easier to understand and also might reduced possible bugs because of unit checking.

My use case in high-level is to convert data from input form (user inputs) and just calculate overall specific thermal resistance from multiple input values and ensure units are handled correctly. End-result is returned in SI form because rest of program uses just plain doubles everywhere. Using this library helped me to write correct code directly and using self documenting code. Calculations between units is very nice feature and makes life easy.

I might extend usage of this library to process other inputs as well and when results are shown back to user in specific units, now all unit logic is some kind ad-hoc code here and there.

I don't have any plans to use this library everywhere as there might be too great performance hit but processing inputs and outputs performance doesn't matter but ensure unit correctness is more important factor.

MadsKirkFoged commented 2 years ago

I had the same worry about performance in the past however the for the projects I work in, the performance between a double and a Unit is not too big. Also we are getting way fewer bug and much better throughput, so it is much cheaper for us to buy bigger server then spent weeks fighting a Unit-mismatch :-)

ikijano commented 2 years ago

Good to know. When I have more time I really need to investigate performance difference and make conclusions after that :)

ikijano commented 2 years ago

This is almost off-topic already, but I made quick performance test for code where SpecificThermalResistance is involved. I duplicated code to use doubles in SI values then run performance test for both versions, so basically it's calculation (with and without units) performance test. Note this part of program has no real hot-path so it doesn't matter if I use slower approach, but I don't think I would use library in other part of program which do heavy calculations. Only when preparing data to desired units.

Test result using my old laptop:

BenchmarkDotNet=v0.13.1, OS=Windows 10.0.19043.1526 (21H1/May2021Update) Intel Core i7-4600U CPU 2.10GHz (Haswell), 1 CPU, 4 logical and 2 physical cores .NET SDK=6.0.100-rc.2.21505.57 [Host] : .NET 5.0.11 (5.0.1121.47308), X64 RyuJIT DefaultJob : .NET 5.0.11 (5.0.1121.47308), X64 RyuJIT

Method Mean Error StdDev Median Gen 0 Allocated
CalculateWithEngineeringUnits 245,042.1 ns 11,493.97 ns 30,279.71 ns 235,369.9 ns 91.7969 192,743 B
CalculateWithoutEngineeringUnits 179.0 ns 2.41 ns 2.25 ns 178.2 ns - -
MadsKirkFoged commented 2 years ago

We could work on trying to optimize for performance (which I have so fare not spent a second on..) Ill make a post about that! :-)