Skip to content

Fix: Missing Temperature / Temperature Operator #340

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
wants to merge 2 commits into from
Closed

Fix: Missing Temperature / Temperature Operator #340

wants to merge 2 commits into from

Conversation

0xferit
Copy link
Contributor

@0xferit 0xferit commented Dec 11, 2017

  • Fixed missing temperature / temperature operator

  • Fixed obsolete call to .DegreeCelciusDelta to .DegreeCelcius in the statement return LapseRate.FromDegreesCelciusPerKilometer(left.DegreesCelsius / right.Kilometers);

@0xferit 0xferit requested a review from angularsen December 11, 2017 16:23
@@ -73,6 +73,11 @@ public partial struct Temperature
{
return new TemperatureDelta(left.Kelvins - right.Kelvins);
}

public static double operator /(Temperature left, Temperature right)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure you don't mean to divide by a temperature delta?

This is tricky and not super intuitive to me, but I have realized that different temperature units have different scales and arithmetic on those are tricky since we talk about degrees Celsius both as a point on the scale as well as a range on the scale, although you can't add two points together since they may have different zero points so you add a delta to a point.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, so you didn't implement Temperature / Temperature on purpose?

What else can we do? Would Temperature / TemperatureDelta be correct?

We are trying to obtain reduced temperature by diving actual temperature to critical temperature. By doing that, I noticed that / operator is missing.

@gratestas knows better than me, perhaps he can explain better.

Copy link
Contributor

@gratestas gratestas Dec 13, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi,
actually it should not be a problem dividing two values of the same quantity with each other as far as both are considered to be on the same scale. And this goes the same way as solid pattern for all arithmetical operations applied for two different points on the phase diagram of any substance for any unit base. Naturally, no one would feel a need to divide two points of a quantity before getting units normalized.
Concerning our case, we need to use a reduced state of a substance so that we can proceed on "standardized"/dimensionless plane, where the analysis of the most substances, if not all, can be treated more easily. In thermodynamics, a reduced state is a normalization of a point with respect to critical point of a substance. And it is very useful way to analyze real behavior of a substance with respect to its ideal state.

So i think @ferittuncer does not have wrong sense.

@angularsen maybe we misunderstood you statement. Maybe you should explain us a little bit more what you meant so that we can talk about the same thing

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is some context on why TemperatureDelta was added in the first place:
#218

Dividing 20 Celsius with 5 Celsius is as simple as 20/5=4, since they are the exact same scale. Dividing 20 Celsius with 5 Fahrenheit requires some conversion because they have different zero points (0 C != 0 F) as well as different slopes (increasing with 1C is different than increasing with 1F).

actually it should not be a problem dividing two values of the same quantity with each other as far as both are considered to be on the same scale.

I think you nailed the key issue here. When you have a Temperature in UnitsNet, that represents a temperature that is not yet tied to a particular unit or scale - not until you convert it to Celsius or Fahrenheit.

Internally a Temperature is represented with Kelvins and arithmetic is performed using that scale, however Kelvin has a different zero point than both Celsius and Fahrenheit This is why arithmetic is tricky with temperature, because adding or dividing two temperatures will be done on Kelvin scale even if you are really working with Celsius or Fahrenheit - so it will not match your expectations.

This is why we added TemperatureDelta to support arithmetic that is agnostic of the scale. A Temperature represents a measurement point and TemperatureDelta represents a range or interval. You add a delta to a temperature, and you multiply or divide a temperature by a delta.

So back to your example.

Naturally, no one would feel a need to divide two points of a quantity before getting units normalized.

I agree dividing two points in the same scale is fine, but doesn't that mean you should be doing something like this?

Temperature temp, criticalTemp; // Values retrieved from somewhere
Temperature normalizedTemp = Temperature.FromDegreesCelsius(temp.DegreesCelsius / criticalTemp.DegreesCelsius);

Does this help or am I maybe still not understanding what you want to achieve?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm satisfied with the explanation. Thank you.

I'm closing this pull request.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I got your point. And I am okay with that too.

thanks again

@0xferit 0xferit closed this Dec 18, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants