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

Update to new open-source FIELD_API #70

Merged
merged 9 commits into from
Feb 7, 2024
Merged

Conversation

awnawab
Copy link
Contributor

@awnawab awnawab commented Feb 5, 2024

This PR updates the scc-field variant to use the new open-source FIELD_API. Notable changes are the use of the FieldGang feature to offload packed fields as a single contiguous entity, and the use of raw device pointers rather than host-mapped device pointers.

@awnawab awnawab requested review from reuterbal and mlange05 February 5, 2024 09:28
Copy link
Collaborator

@reuterbal reuterbal left a comment

Choose a reason for hiding this comment

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

Thanks, this looks great and very much looking forward to switching to the open-source field-api repository! Currently, the NVHPC tests fail to compile field-api and it would be good to get this sorted.

bundle.yml Outdated
ENABLE_FIELD_API_TESTS=OFF
ENABLE_FIELD_API_FIAT_BUILD=OFF
FIELD_API_UTIL_MODULE_PATH=${CMAKE_SOURCE_DIR}/cloudsc-dwarf/src/common/module
ENABLE_SINGLE_PRECISION=OFF
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since this is effectively a global option in this form, we might want to simply put this at the top of the bundle.yml.

@awnawab awnawab force-pushed the naan-update-field_api branch 3 times, most recently from f2713f2 to 72ca253 Compare February 5, 2024 13:20
@awnawab
Copy link
Contributor Author

awnawab commented Feb 5, 2024

Thanks for the review @reuterbal and for all the offline discussion. I am converting this to a draft since I will need a FIELD_API build-system update to get around building with acc=host and cuda (for the CLOUSC CI).

@awnawab awnawab marked this pull request as draft February 5, 2024 15:57
@awnawab awnawab force-pushed the naan-update-field_api branch 2 times, most recently from d618af0 to 4d3e726 Compare February 5, 2024 16:49
@awnawab awnawab force-pushed the naan-update-field_api branch from 4d3e726 to d66b5f5 Compare February 5, 2024 16:52
@awnawab
Copy link
Contributor Author

awnawab commented Feb 5, 2024

Thanks again @reuterbal for all the help with this. The CI now builds the nvhpc runs with the -acc=gpu flag i.e. how we intend to build them for use. Because this inserts cuInit calls, we can't run these on the CI yet. Therefore, a separate nvhpc run is added with OpenACC explicitly disabled to test the loki and gpu variants (similar to the GNU CI runs).

@awnawab awnawab marked this pull request as ready for review February 5, 2024 17:43
Copy link
Collaborator

@reuterbal reuterbal left a comment

Choose a reason for hiding this comment

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

I think that's a reasonable solution retaining coverage for some variants built with nvhpc and testing at least the build for the others. Many thanks for grinding through this!

@reuterbal
Copy link
Collaborator

As discussed: Merging now and let's have a 1-line change PR once the new FIELD_API version has been tagged!

@reuterbal reuterbal merged commit 6961e75 into develop Feb 7, 2024
18 checks passed
@reuterbal reuterbal deleted the naan-update-field_api branch February 7, 2024 12:33
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