Temperature unit#639
Conversation
|
Reminder to add simple unit test |
| : UnitSystem(std::forward<T>(t), eos_units_init::thermal_units_init_tag, | ||
| (length_unit * length_unit * length_unit) / mass_unit, | ||
| (time_unit * time_unit) / (length_unit * length_unit), temp_unit) {} | ||
| (time_unit * time_unit) / (length_unit * length_unit), 1.0 / temp_unit) {} |
There was a problem hiding this comment.
relative to the densithy and sie, this looks correct. Users may need to make changes on their end.
There was a problem hiding this comment.
Yeah, just like the original commit that changed this, this will be breaking
Yurlungur
left a comment
There was a problem hiding this comment.
Thanks for the fix. I think this was an incomplete fix from before. I won't block the merge, but should we add a test? If not, just push back and I'll click merge.
Test added. I pulled the exact values from Artemis that were causing the issue. If you switch |
|
Assuming tests are passing on re-git, merge when ready |
Dealing with an issue in Artemis, flipping this fixes it. I think this is consistent with how the mass, time, and length units are interpreted in this function.
PR Summary
PR Checklist
make formatcommand after configuring withcmake.plan_historiesfolder, with a filename the same as the MR number.If preparing for a new release, in addition please check the following: