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

GPU PiecewiseLinearTwoPhaseMaterialParams maintenance and bugfix #4403

Merged

Conversation

multitalentloes
Copy link
Contributor

@multitalentloes multitalentloes commented Jan 3, 2025

Finalization is adjusted to avoid the cpu accessing values from gpumemory, in particular, a mutable buffer. Avoiding the finalization for the GPU instances is less robust, solutions to this are welcome.

move_to_gpu is added to PiecewiseLinearTwoPhaseMaterialParams to make managament of GPU instances easier, and design more consistent with other classes.

@multitalentloes multitalentloes changed the title add move_to_gpu and adjust finalization GPU PiecewiseLinearTwoPhaseMaterialParams maintenance and bugfix Jan 3, 2025
@kjetilly
Copy link
Contributor

kjetilly commented Jan 7, 2025

Jenkins build this please

Finalization is adjusted to avoid the cpu
accessing values from gpumemory, in particular
a mutable buffer.
@kjetilly
Copy link
Contributor

kjetilly commented Jan 8, 2025

Jenkins build this please

Copy link
Contributor

@kjetilly kjetilly left a comment

Choose a reason for hiding this comment

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

Just some nitpicking comments.

@multitalentloes multitalentloes force-pushed the fix_gpu_linear_two_phase_material branch from 5a217b2 to ea29426 Compare January 8, 2025 14:09
@multitalentloes
Copy link
Contributor Author

jenkins build this opm-simulators=5838 please

Copy link
Contributor

@kjetilly kjetilly left a comment

Choose a reason for hiding this comment

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

This looks fine to me. I await approval from @bska before merging.

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.

My concerns have been fully addressed. This looks good to me.

@kjetilly
Copy link
Contributor

Jenkins build this please

@kjetilly
Copy link
Contributor

jenkins build this opm-simulators=5838 please

@kjetilly kjetilly merged commit 9851aff into OPM:master Jan 13, 2025
1 check passed
@akva2
Copy link
Member

akva2 commented Jan 13, 2025

next time, you should not merge anything with commit messages like 'WIP'.

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.

4 participants