-
Notifications
You must be signed in to change notification settings - Fork 393
New Quantity: SpecificEntropy #322
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
New Quantity: SpecificEntropy #322
Conversation
I reverted |
detected an error during convertion from kilocalories to joule unit base
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.
Everything looks fine except consistency in scientific notation. AFAIK we prefer using lower-case e
without zero padding.
"Name": "SpecificEntropy", | ||
"BaseUnit": "JoulePerKilogramKelvin", | ||
"XmlDoc": "The SpecificEntropy", | ||
"XmlDocRemarks": "Specific entropy is an amount of energy required to raise temperature of a substance by 1 Kelvin per unit mass", |
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.
XmlDocRemarks is not supported, move this into XmlDoc.
"SingularName": "CaloriePerGramKelvin", | ||
"PluralName": "CaloriesPerGramKelvin", | ||
"FromUnitToBaseFunc": "x*4.184e3", | ||
"FromBaseToUnitFunc": "x/4.184e3", |
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 2.390057e-4
at: https://www.gordonengland.co.uk/conversion/specific_heat.htm
And 0.0002390057361377
at https://www.translatorscafe.com/unit-converter/en/specific-heat-capacity/1-6/joule%2Fkilogram%2FK-calorie%20(IT)%2Fgram%2F%C2%B0C/
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.
Just asking to prevent any potential misunderstanding: Do you suggest to change the 2.39005e-4
with what you got, 2.390057e-4
, in SpecificEntropyTests.cs
?
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 would use the must accurate value found in a reference source 0.0002390057361377
in the test.
Then these JSON functions must also be updated as they seem wrong, but I don't know if they should be represented as some calculation or as the same constant.
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 couldn't spot the error in JSON function :/
Fixed values in tests.
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.
BTW, https://www.translatorscafe.com/unit-converter/en/specific-heat-capacity/1-6/joule%2Fkilogram%2FK-calorie%20(IT)%2Fgram%2F%C2%B0C/ gives 0.000238845896627
not 0.0002390057361377
.
I updated values in the test as 0.0002390057
.
I would like to leave this p.r. to you and @gratestas to finalize before merging, as I'm a bit confused.
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.
Careful, you linked to the IT calorie unit. It is slightly different than Th
calories. I think we should add both and name them explicitly to avoid ambiguity.
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.
Oh, yeah, but I copied your link :)
I agree that we should fix this ambiguity in the entire library soon.
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.
Sorry, my bad. The JSON functions are probably good, at least they match the constant pretty well.
I changed the abbreviations according to https://www.princeton.edu/~maelabs/mae324/glos324/entropy.htm |
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.
As far as I can tell, this looks good to go.
Nice, @ferittuncer do you want to do the honors of releasing a new nuget? |
Sure @angularsen, 3.80.0 is on its way! :) |
Wohoo! :-) |
@ferittuncer I noticed you didn't push the git tag If you just want to push tags, and not code, you can run If you could run either of these two commands to push the tag you have locally, that would be great! |
Thanks for information. I am going to do just as you described next time @angularsen. I see that you released 3.81.0 now, so do I have to take any action for now? |
Yes, please push tag 3.80.0 so that the repository has a tag for the release. I could create the tag, but then you and me/others would have different versions of the same tag.
The tags then show under Releases: https://github.com/angularsen/UnitsNet/releases I also try to add the changes to the tag text, but I've not been strict about this and often forgot or didn't bother. The advantage is that the Releases page would list the changes per release. This can be edited from the github website too. |
@angularsen I did push the tags but it seems I pushed a wrong tag mistakenly too. Sorry, I'm not experienced with tags. What should we do about this? 7bdd0f0 This tag was created in my local repo when I mistakenly bumped JsonNet version, I dropped that commit but it seems the tag remained, so when I pushed all the tags, you know... |
No biggie, just delete the tag both locally and on remote.
The only thing is that if anyone else has pulled from the repo, they will also have the tag locally and if they are a collaborator they will re-push the same tag back up again. Git is a bit messy on these things. |
Yup, tags corrected. : ) |
No description provided.