Skip to content

Utilize PressureSlope unit in Media consistently #4262

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Jan 16, 2024
Merged

Utilize PressureSlope unit in Media consistently #4262

merged 2 commits into from
Jan 16, 2024

Conversation

@beutlich
Copy link
Member

@beutlich beutlich commented Jan 14, 2024

This is a follow-up of the recently merged PRs #4115 (adding unit "Pa/s") and #4103 (introducing a new type PressureRate with unit "Pa/s").

@beutlich beutlich added the L: Media Issue addresses Modelica.Media label Jan 14, 2024
@beutlich beutlich added this to the MSL4.1.0 milestone Jan 14, 2024
@beutlich beutlich force-pushed the utilize-PressureRate-unit branch from 770cbc2 to d6fd498 Compare January 14, 2024 19:49
@beutlich beutlich changed the title Utilize PressureRate unit in Media everywhere Utilize PressureRate unit in Media consistently Jan 14, 2024
@beutlich beutlich enabled auto-merge (squash) January 14, 2024 19:50
@beutlich beutlich force-pushed the utilize-PressureRate-unit branch from d6fd498 to 5c48e64 Compare January 15, 2024 05:55
maltelenz
maltelenz approved these changes Jan 15, 2024
View reviewed changes
Copy link
Contributor

@maltelenz maltelenz left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@HansOlsson
Copy link
Contributor

HansOlsson commented Jan 15, 2024 •
edited
Loading

Based on #4236 shouldn't it be PressureSlope instead of PressureRate?

That would make Pressure and Temperature more consistent.
Based on #4236 FlowRate is for connector variables, whereas Slope is for derivatives, (and plain Rate is used for completely different things).

@beutlich
Copy link
Member Author

beutlich commented Jan 15, 2024

Based on #4236 shouldn't it be PressureSlope instead of PressureRate?

That would make Pressure and Temperature more consistent. Based on #4236 FlowRate is for connector variables, whereas Slope is for derivatives, (and plain Rate is used for completely different things).

Will do (in this PR).

@beutlich beutlich disabled auto-merge January 15, 2024 17:03
@beutlich beutlich changed the title Utilize PressureRate unit in Media consistently Utilize PressureSlope unit in Media consistently Jan 15, 2024
@beutlich beutlich force-pushed the utilize-PressureRate-unit branch from 6ab5d04 to ab8ef97 Compare January 15, 2024 19:30
casella
casella approved these changes Jan 16, 2024
View reviewed changes
Copy link
Contributor

@casella casella left a comment

Choose a reason for hiding this comment

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

LGTM

@casella casella merged commit fa90ce7 into modelica:master Jan 16, 2024
@beutlich beutlich deleted the utilize-PressureRate-unit branch January 16, 2024 21:56
@beutlich beutlich mentioned this pull request Jan 17, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@maltelenz maltelenz maltelenz approved these changes

@casella casella casella approved these changes

Assignees

No one assigned

Labels

L: Media Issue addresses Modelica.Media

Projects

None yet

Milestone

MSL4.1.0

Development

Successfully merging this pull request may close these issues.

4 participants

@beutlich @HansOlsson @maltelenz @casella