Skip to content
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

adding ZMF keyword #4200

Merged
merged 1 commit into from
Oct 3, 2024
Merged

adding ZMF keyword #4200

merged 1 commit into from
Oct 3, 2024

Conversation

GitPaean
Copy link
Member

@GitPaean GitPaean commented Sep 16, 2024 •
edited
Loading

The keyword ZMF is used to specify the mole fraction of a component per cell. It is a keyword for SOLUTION section.

@GitPaean GitPaean force-pushed the adding_ZMF branch 2 times, most recently from 7a29150 to d702e2d Compare September 20, 2024 14:01
@GitPaean GitPaean mentioned this pull request Sep 23, 2024
Merged
@GitPaean GitPaean marked this pull request as ready for review September 30, 2024 08:20
@GitPaean GitPaean marked this pull request as draft September 30, 2024 08:20
@GitPaean GitPaean force-pushed the adding_ZMF branch 2 times, most recently from 11f5e07 to 928b7ac Compare October 2, 2024 07:52
@GitPaean
Copy link
Member Author

GitPaean commented Oct 2, 2024

jenkins build this opm-simulators=5625 please

@GitPaean GitPaean marked this pull request as ready for review October 2, 2024 10:20
@GitPaean
Copy link
Member Author

GitPaean commented Oct 2, 2024

marking this as ready for review.

@GitPaean GitPaean requested a review from bska October 2, 2024 10:20
@GitPaean
Copy link
Member Author

GitPaean commented Oct 2, 2024

jenkins build this opm-simulators=5625 please

@GitPaean GitPaean force-pushed the adding_ZMF branch 2 times, most recently from 57a0273 to 6cd8c93 Compare October 2, 2024 11:51
@GitPaean
Copy link
Member Author

GitPaean commented Oct 2, 2024

jenkins build this opm-simulators=5625 please

bska
bska reviewed Oct 2, 2024
View reviewed changes
Copy link
Member

@bska bska left a comment

Choose a reason for hiding this comment

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

It's not clear to me what we gain by the API change in phaseIsActive(). Other than that, this looks good to me with the possible exception that the new comment in CompositionalTests.cpp mostly speaks to those who are very familiar with implementation.

opm/material/fluidsystems/GenericOilGasFluidSystem.hpp Outdated Show resolved Hide resolved
tests/parser/CompositionalTests.cpp Outdated Show resolved Hide resolved
@GitPaean
Copy link
Member Author

GitPaean commented Oct 2, 2024

Thanks for the comments. I think I have addressed them.

@GitPaean
Copy link
Member Author

GitPaean commented Oct 2, 2024

jenkins build this opm-simulators=5625 please

@bska
Copy link
Member

bska commented Oct 3, 2024

opm-simulators=5625

Just a question on this aspect: Must the two PRs be merged together or can this be reviewed/merged independently and then the downstream one in a separate operation?

@GitPaean
Copy link
Member Author

GitPaean commented Oct 3, 2024

Just a question on this aspect: Must the two PRs be merged together or can this be reviewed/merged independently and then the downstream one in a separate operation?

I think this PR can go in first.

@GitPaean
Copy link
Member Author

GitPaean commented Oct 3, 2024

jenkins build this please

bska
bska approved these changes Oct 3, 2024
View reviewed changes
Copy link
Member

@bska bska left a comment

Choose a reason for hiding this comment

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

Thanks a lot for the updates. This looks good to me now.

I think this PR can go in first.

Much appreciated. In that case I'll merge this into the master branch.

@bska bska merged commit 740e05a into OPM:master Oct 3, 2024
1 check passed
@GitPaean GitPaean deleted the adding_ZMF branch October 4, 2024 08:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@akva2 akva2 akva2 left review comments

@bska bska bska approved these changes

Assignees
No one assigned
Labels
need manual update
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@GitPaean @bska @akva2