-
Notifications
You must be signed in to change notification settings - Fork 396
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
Closed
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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).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. ATemperature
represents a measurement point andTemperatureDelta
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.
I agree dividing two points in the same scale is fine, but doesn't that mean you should be doing something like this?
Does this help or am I maybe still not understanding what you want to achieve?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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