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

Temperature delta base dimensions? #777

Closed
lipchev opened this issue Apr 6, 2020 · 6 comments · Fixed by #840
Closed

Temperature delta base dimensions? #777

lipchev opened this issue Apr 6, 2020 · 6 comments · Fixed by #840
Labels

Comments

@lipchev
Copy link
Collaborator

lipchev commented Apr 6, 2020

As a prelude to #709 I figure I'd run a test of the currently defined operators and see if there is mismatch between the dimensions of the operands and the result. To my great satisfaction- it found the faulty operator for Density / Mass (which I had previously annotated as obsolete- and completely forgotten about). The only other problem was with the TemperatureDelta which has no defined BaseDimensions- thus failing the dimensions multiplication (though I assume the operators are correct). Was there a reason for it not having the Temperature dimension defined?
Here is the test (I'd put it in the GeneratedQuantityCodeTests):


            [Fact]
            public void HasMultiplicationOperator_GivenMassAndVolume_ReturnsFalse()
            {
                Assert.False(HasMultiplicationOperator(typeof(Mass), typeof(Volume)));
                Assert.DoesNotContain(typeof(Volume), GetMultipliers(typeof(Mass)));
            }

            [Fact]
            public void HasMultiplicationOperator_GivenDensityAndVolume_ReturnsTrue()
            {
                Assert.True(HasMultiplicationOperator(typeof(Density), typeof(Volume)));
                Assert.Contains(typeof(Volume), GetMultipliers(typeof(Density)));
                Assert.Equal(typeof(Mass), GetMultiplicationResult(typeof(Density), typeof(Volume)));
            }

            [Fact]
            public void HasDivisionOperator_GivenDensityAndVolume_ReturnsFalse()
            {
                Assert.False(HasDivisionOperator(typeof(Density), typeof(Volume)));
                Assert.DoesNotContain(typeof(Volume), GetDivisors(typeof(Density)));
            }

            [Fact]
            public void HasDivisionOperator_GivenMassAndVolume_ReturnsTrue()
            {
                Assert.True(HasDivisionOperator(typeof(Mass), typeof(Volume)));
                Assert.Contains(typeof(Volume), GetDivisors(typeof(Mass)));
                Assert.Equal(typeof(Density), GetDivisionResult(typeof(Mass), typeof(Volume)));
            }

            [Fact]
            public void HasMultiplicationOperator_GivenTwoQuantities_ReturnsTrueIfDimensionsMultiplicationIsValid()
            {
                var failed = new List<Tuple<QuantityInfo, QuantityInfo, QuantityInfo>>();
                foreach (QuantityInfo firstQuantity in Quantity.Infos)
                {
                    foreach (Type divisor in GetMultipliers(firstQuantity.ValueType))
                    {
                        QuantityInfo secondQuantity = Quantity.Infos.FirstOrDefault(x => x.ValueType == divisor);
                        if (secondQuantity == null)
                        {
                            continue; // scalers
                        }
                        BaseDimensions resultDimensions = firstQuantity.BaseDimensions * secondQuantity.BaseDimensions;
                        Type resultingType = GetMultiplicationResult(firstQuantity.ValueType, secondQuantity.ValueType);
                        QuantityInfo resultQuantity = Quantity.Infos.FirstOrDefault(x => x.ValueType == resultingType);
                        if (resultQuantity == null)
                        {
                            continue; // scalers
                        }
                        if (resultQuantity.BaseDimensions != resultDimensions)
                        {
                            failed.Add(new Tuple<QuantityInfo, QuantityInfo, QuantityInfo>(firstQuantity, secondQuantity, resultQuantity));
                        }
//                        Assert.Equal(resultQuantity.BaseDimensions, resultDimensions);
                    }
                }
                Assert.Empty(failed);
            }

            [Fact]
            public void HasDivisionOperator_GivenTwoQuantities_ReturnsTrueIfDimensionsDivisionIsValid()
            {
                var failed = new List<Tuple<QuantityInfo, QuantityInfo, QuantityInfo>>();
                foreach (QuantityInfo firstQuantity in Quantity.Infos)
                {
                    foreach (Type divisor in GetDivisors(firstQuantity.ValueType))
                    {
                        QuantityInfo secondQuantity = Quantity.Infos.FirstOrDefault(x => x.ValueType == divisor);
                        if (secondQuantity == null)
                        {
                            continue; // scalers
                        }
                        BaseDimensions resultDimensions = firstQuantity.BaseDimensions / secondQuantity.BaseDimensions;
                        Type resultingType = GetDivisionResult(firstQuantity.ValueType, secondQuantity.ValueType);
                        QuantityInfo resultQuantity = Quantity.Infos.FirstOrDefault(x => x.ValueType == resultingType);
                        if (resultQuantity == null)
                        {
                            continue; // scalers
                        }
                        if (resultQuantity.BaseDimensions != resultDimensions)
                        {
                            failed.Add(new Tuple<QuantityInfo, QuantityInfo, QuantityInfo>(firstQuantity, secondQuantity, resultQuantity));
                        }
//                        Assert.Equal(resultQuantity.BaseDimensions, resultDimensions);
                    }
                }
                Assert.Empty(failed);
            }

            private static bool HasMultiplicationOperator(Type t, Type operandType)
            {
                var operation = t.GetMethod("op_Multiply", new[] { t, operandType });
                return operation != null && operation.IsSpecialName;
            }

            private static bool HasDivisionOperator(Type t, Type operandType)
            {
                var operation = t.GetMethod("op_Division", new[] { t, operandType });
                return operation != null && operation.IsSpecialName;
            }

            private static Type GetMultiplicationResult(Type t, Type operandType)
            {
                var operation = t.GetMethod("op_Multiply", new[] { t, operandType });
                return operation != null && operation.IsSpecialName ? operation.ReturnType : null;
            }

            private static Type GetDivisionResult(Type t, Type operandType)
            {
                var operation = t.GetMethod("op_Division", new[] { t, operandType });
                return operation != null && operation.IsSpecialName ? operation.ReturnType : null;
            }

            private static IEnumerable<Type> GetMultipliers(Type t)
            {
                return t.GetMethods().Where(x => x.IsSpecialName && x.Name == "op_Multiply" && x.CustomAttributes.All(a => a.AttributeType != typeof(ObsoleteAttribute)))
                    .SelectMany(x => x.GetParameters().Skip(1).Select(p => p.ParameterType));
            }

            private static IEnumerable<Type> GetDivisors(Type t)
            {
                return t.GetMethods().Where(x => x.IsSpecialName && x.Name == "op_Division" && x.CustomAttributes.All(a => a.AttributeType != typeof(ObsoleteAttribute)))
                    .SelectMany(x => x.GetParameters().Skip(1).Select(p => p.ParameterType));
            }
@lipchev lipchev added the bug label Apr 6, 2020
@lipchev
Copy link
Collaborator Author

lipchev commented Apr 6, 2020

Oops, I just now remembered that I'm on an older version of UnitsNet- so, there might be something new by now...

@stale
Copy link

stale bot commented Jun 5, 2020

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 Jun 5, 2020
@lipchev
Copy link
Collaborator Author

lipchev commented Jun 5, 2020

@angularsen, @tmilnthorp - hey guys- could you please tell me if you know of any reason for the TemperatureDelta quantity not having defined BaseDimensions. If it is a bug- then I will gladly fix it- so that can I move on with a PR for adding those operator tests that I mentioned up top.
Cheers

@stale stale bot removed the wontfix label Jun 5, 2020
@angularsen
Copy link
Owner

angularsen commented Jun 6, 2020

@tmilnthorp is probably best suited to answer this, I'm not really sure.

The idea with BaseDimensions if I recall correctly, is to be able to determine from a computation of multiplying or dividing two IQuantitys that the resulting dimensions represents a Temperature or some other quantity type. We could then also use this to automatically generate the code for all the possible operator overloads between our quantities.

I'm not sure there is a way to multiply or divide two quantities, where the expected result is a TemperatureDelta? In a way, I think TemperatureDelta is not really a separate quantity as much as it just represents a different (relative) scale of Temperature.

If so, then maybe it shouldn't have BaseDimensions.

@stale
Copy link

stale bot commented Aug 5, 2020

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.

@lipchev
Copy link
Collaborator Author

lipchev commented Sep 21, 2020

For the record- there is the TemperatureDelta operator *(LapseRate left, Length right)

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

Successfully merging a pull request may close this issue.

2 participants