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.

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

Add a container for FLOWS data output #5954

Merged
merged 10 commits into from
Feb 18, 2025
Merged

Add a container for FLOWS data output #5954

merged 10 commits into from
Feb 18, 2025

Conversation

akva2
Copy link
Member

@akva2 akva2 commented Feb 4, 2025

Sits on top of #5950

@akva2
Copy link
Member Author

akva2 commented Feb 4, 2025

jenkins build this please

@akva2
Copy link
Member Author

akva2 commented Feb 4, 2025

jenkins build this opm-common=4460 please

@akva2 akva2 marked this pull request as ready for review February 6, 2025 14:35
@akva2 akva2 marked this pull request as draft February 6, 2025 14:35
@bska
Copy link
Member

bska commented Feb 13, 2025

Sits on top of #5950

That PR was merged into master. Is this work ready for review now, @akva2 ?

@akva2
Copy link
Member Author

akva2 commented Feb 13, 2025

as such, yes. however, there is incoming work in OPM/opm-common#4410 that will affect this. in particular I can simplify this as there is no longer a -1 index for the water phase in the generic fluid system, which means i remove some additional template stuff I had to do to avoid compiler warnings. I think we should wait for that and then I can redo.

@bska
Copy link
Member

bska commented Feb 13, 2025

there is incoming work in OPM/opm-common#4410 that will affect this. [...] I think we should wait for that and then I can redo.

Noted. Thanks a lot for the information.

@akva2 akva2 marked this pull request as ready for review February 14, 2025 09:39
@akva2
Copy link
Member Author

akva2 commented Feb 14, 2025

turns out the component index is still -1 for water, only the phase index changes, so i couldn't really simplify things. but in any case, this is now ready for review.

@akva2
Copy link
Member Author

akva2 commented Feb 14, 2025

jenkins build this please

@akva2 akva2 force-pushed the flows_container branch 2 times, most recently from 19de675 to 2c43e50 Compare February 18, 2025 07:27
@akva2
Copy link
Member Author

akva2 commented Feb 18, 2025

jenkins build this please

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.

Excellent. I think this is ready for merging once the final tiny issue has been addressed.

@akva2
Copy link
Member Author

akva2 commented Feb 18, 2025

jenkins build this please

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 and I'll merge into master.

@bska bska merged commit 0cd4d8e into OPM:master Feb 18, 2025
1 check passed
@akva2 akva2 deleted the flows_container branch February 18, 2025 09:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants