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

homme SYCL changes #6594

Merged
merged 12 commits into from
Sep 21, 2024
Merged

homme SYCL changes #6594

merged 12 commits into from
Sep 21, 2024

Conversation

oksanaguba
Copy link
Contributor

@oksanaguba oksanaguba commented Sep 6, 2024

Changes needed for spot/aurora: sycl kokkos spaces, temporary TP changes (TP init will be sorted out later), switch booleans to ints in F<-->xx interfaces.

see #6569 for the original branch/history.

[bfb]

@oksanaguba oksanaguba self-assigned this Sep 6, 2024
Copy link

github-actions bot commented Sep 6, 2024

PR Preview Action v1.4.7
🚀 Deployed preview to https://E3SM-Project.github.io/E3SM/pr-preview/pr-6594/
on branch gh-pages at 2024-09-17 01:06 UTC

@ambrad
Copy link
Member

ambrad commented Sep 6, 2024

As I understand it, the int/bool code is just to work around a compiler bug affecting the F90-C++ interface; essentially, the compiler's treatment of c_bool is broken, or something like that. In particular, using bool in a kernel is not an issue; we do that in several spots. Oksana, is all of that correct?

If so, I can add a commit to this branch next week that isolates the workaround to just the F90-C++ interface code. In particular, I'd like to remove the two theta_hydrostatic_mode variables from control_mod.F90. Is that OK?

@oksanaguba
Copy link
Contributor Author

As I understand it, the int/bool code is just to work around a compiler bug affecting the F90-C++ interface; essentially, the compiler's treatment of c_bool is broken, or something like that. In particular, using bool in a kernel is not an issue; we do that in several spots. Oksana, is all of that correct?

Yes, correct about int-bool issue.

If so, I can add a commit to this branch next week that isolates the workaround to just the F90-C++ interface code. In particular, I'd like to remove the two theta_hydrostatic_mode variables from control_mod.F90. Is that OK?

yes, thanks!

@oksanaguba oksanaguba mentioned this pull request Sep 6, 2024
@rljacob
Copy link
Member

rljacob commented Sep 6, 2024

@bartgol thank for reducing the number of commits but you could do this one more time using --author on the commit command to give author credit to Oksana?

@bartgol
Copy link
Contributor

bartgol commented Sep 6, 2024

Of course. I'll do it as soon as I get back to my computer

@bartgol
Copy link
Contributor

bartgol commented Sep 6, 2024

@rljacob @oksanaguba I edited the commits, to fix the author.

@ambrad FYI, in case you pulled and was planning to do the mods you mentioned...

@ambrad
Copy link
Member

ambrad commented Sep 9, 2024

I think I'll have a commit to push to this branch by tonight. I've made the changes and am running homme_integration on Chrysalis.

Also fix preqx's use of use_moisture. Remove MOIST-DRY enum to avoid confusion,
since it's no longer used.
@ambrad
Copy link
Member

ambrad commented Sep 9, 2024

Done. I also fixed some moisture-related code in preqx so that homme_integration passes on Chrysalis.

@oksanaguba
Copy link
Contributor Author

homme suite pass on chrysalis

oksanaguba added a commit that referenced this pull request Sep 20, 2024
Changes needed for spot/aurora: sycl kokkos spaces,
temporary TP changes (TP init will be sorted out later),
switch booleans to ints in F<-->xx interfaces.

see #6569 for the original branch/history.

[bfb]
@oksanaguba
Copy link
Contributor Author

in next

@oksanaguba
Copy link
Contributor Author

hommexx ers_d test failed on cdash with an mpi error, but i ran it by hand with no issues.

@oksanaguba oksanaguba merged commit ac05a69 into master Sep 21, 2024
5 checks passed
@oksanaguba oksanaguba deleted the bartgol/homme/sycl branch September 21, 2024 20:37
ambrad added a commit to ambrad/E3SM that referenced this pull request Sep 24, 2024
These are due to PR E3SM-Project#6594 and break the E3SM-repo EAMxx build. Once the SCREAM
and E3SM repos are unified, we can back out these workarounds.

The workarounds are isolated to components/homme, keeping the commit separation
of components/eamxx (SCREAM repo) and components/homme (E3SM repo) clean.
ambrad added a commit that referenced this pull request Sep 24, 2024
…6645)

Hommexx: Temporarily work around some EAMxx-Hommexx incompatibilities.

These are due to PR #6594 and break the E3SM-repo EAMxx build. Once the SCREAM
and E3SM repos are unified, we can back out these workarounds.

The workarounds are isolated to components/homme, keeping the commit separation
of components/eamxx (SCREAM repo) and components/homme (E3SM repo) clean.

Fixes #6635.

[BFB]
ambrad added a commit that referenced this pull request Sep 25, 2024
Hommexx: Temporarily work around some EAMxx-Hommexx incompatibilities.

These are due to PR #6594 and break the E3SM-repo EAMxx build. Once the SCREAM
and E3SM repos are unified, we can back out these workarounds.

The workarounds are isolated to components/homme, keeping the commit separation
of components/eamxx (SCREAM repo) and components/homme (E3SM repo) clean.

Fixes #6635.

[BFB]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants