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

Enable E3SM shared pi in MPAS-Ocean #6485

Merged
merged 11 commits into from
Oct 23, 2024
Merged

Enable E3SM shared pi in MPAS-Ocean #6485

merged 11 commits into from
Oct 23, 2024

Conversation

cbegeman
Copy link
Contributor

@cbegeman cbegeman commented Jun 24, 2024 •
edited by jonbob
Loading

This PR enables the use of E3SM shared constant pi in MPAS-Ocean when run in coupled configurations.

[non-BFB]

@cbegeman cbegeman requested a review from xylar June 24, 2024 16:49
@cbegeman
Copy link
Contributor Author

@xylar In addition to your #6481, do we want to include the changes I have here as a separate PR?

@rljacob
Copy link
Member

rljacob commented Jun 26, 2024

Does this change answers for coupled cases?

@xylar
Copy link
Contributor

xylar commented Jun 27, 2024

@rljacob, I think these will be answer changing for all runs involving MPAS-Ocean, including coupled runs.

@cbegeman, that is why I had not opted to make these changes yet, though I think they are very desirable.

@xylar xylar added the non-BFB PR makes roundoff changes to answers. label Jun 27, 2024
@cbegeman
Copy link
Contributor Author

Would you like me to close this PR and reopen at a later date?

@xylar
Copy link
Contributor

xylar commented Jun 27, 2024

@cbegeman. let's see what @rljacob says.

@rljacob
Copy link
Member

rljacob commented Jun 27, 2024

Leave it open. We'll be making the maint-3.0 branch soon and this can go in after that.

@rljacob rljacob added this to the v3.1beta milestone Jun 27, 2024
@xylar
Copy link
Contributor

xylar commented Jun 27, 2024

Great, thanks, @rljacob!

@cbegeman
Copy link
Contributor Author

Thanks, @rljacob and @xylar for your feedback

xylar
xylar requested changes Jun 28, 2024
View reviewed changes
Copy link
Contributor

@xylar xylar left a comment •
edited
Loading

Choose a reason for hiding this comment

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

There are still many places in both shared and analysis_members that use pii from the MPAS framework. These should also be updated to use MPAS-Ocean's pi instead. See include:
https://github.com/search?q=repo%3AE3SM-Project%2FE3SM+pii+path%3A%2F%5Ecomponents%5C%2Fmpas-ocean%5C%2F%2F&type=code

This was taken care of for global ocean init mode in #6481

components/mpas-ocean/src/shared/mpas_ocn_constants.F Outdated Show resolved Hide resolved
@cbegeman
Copy link
Contributor Author

cbegeman commented Jul 1, 2024

There are still many places in both shared and analysis_members that use pii from the MPAS framework.

Thanks for pointing this out! I didn't think to grep for that spelling. I'll make those changes as well.

@proteanplanet
Copy link
Contributor

We are planning a separate PR for sea ice.

@cbegeman
Copy link
Contributor Author

cbegeman commented Jul 1, 2024

We are planning a separate PR for sea ice.

Great. I'll confine this PR to only the ocean component then.

@cbegeman cbegeman force-pushed the ocn/enable-e3sm-shared-pi branch from cf5ead7 to 83c5a22 Compare July 1, 2024 20:01
@cbegeman cbegeman force-pushed the ocn/enable-e3sm-shared-pi branch from 83c5a22 to b3c8342 Compare July 1, 2024 20:47
@jonbob
Copy link
Contributor

jonbob commented Oct 21, 2024

@xylar - have your comments been resolved for this PR?

xylar
xylar approved these changes Oct 21, 2024
View reviewed changes
Copy link
Contributor

@xylar xylar left a comment

Choose a reason for hiding this comment

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

@jonbob, yes, thanks this looks great now! @cbegeman, thanks for the hard work!

jonbob added a commit that referenced this pull request Oct 22, 2024
@jonbob
Merge branch 'cbegeman/ocn/enable-e3sm-shared-pi' into next (PR #6485)
b04c94d 
Enable E3SM shared pi in MPAS-Ocean

This PR enables the use of E3SM shared constant pi in MPAS-Ocean when
run in coupled configurations.

[non-BFB]
@jonbob
Copy link
Contributor

jonbob commented Oct 22, 2024

Shows expected baseline DIFFs for:

  • ERP_Ld3.ne30pg2_r05_IcoswISC30E3r5.WCYCL1850.chrysalis_intel.allactive-pioroot1

merged to next

@rljacob
Copy link
Member

rljacob commented Oct 22, 2024

won't this show a diff for any compset with a full ocean?

@cbegeman
Copy link
Contributor Author

won't this show a diff for any compset with a full ocean?

@rljacob Yes

@jonbob
Copy link
Contributor

jonbob commented Oct 22, 2024

@rljacob -- did I misunderstand that the repo was open for non-BFB PRs?

@rljacob
Copy link
Member

rljacob commented Oct 22, 2024

it is.

@jonbob jonbob merged commit accee4c into E3SM-Project:master Oct 23, 2024
12 of 13 checks passed
@jonbob
Copy link
Contributor

jonbob commented Oct 23, 2024

merged to master and expected DIFFs blessed -- except on anvil, which has yet to report

@jonbob
Copy link
Contributor

jonbob commented Oct 23, 2024

also blessed on anvil

xylar added a commit to xylar/compass that referenced this pull request Oct 26, 2024
@xylar
Update E3SM-Project submodule
1167c3c 
This merge updates the E3SM-Project submodule from [727ad81](https://github.com/E3SM-Project/E3SM/tree/727ad81) to [1442143](https://github.com/E3SM-Project/E3SM/tree/1442143).

This update includes the following MPAS-Ocean and MPAS-Frameworks PRs (check mark indicates bit-for-bit with previous PR in the list):
- [ ]  (ocn) E3SM-Project/E3SM#6509
- [ ]  (ocn) E3SM-Project/E3SM#6508
- [ ]  (fwk) E3SM-Project/E3SM#6575
- [ ]  (ocn) E3SM-Project/E3SM#6590
- [ ]  (fwk) E3SM-Project/E3SM#6643
- [ ]  (ocn) E3SM-Project/E3SM#6656
- [ ]  (ocn) E3SM-Project/E3SM#6672
- [ ]  (ocn) E3SM-Project/E3SM#6659
- [ ]  (ocn) E3SM-Project/E3SM#6497
- [ ]  (ocn) E3SM-Project/E3SM#6485
- [ ]  (ocn) E3SM-Project/E3SM#6566
@xylar xylar mentioned this pull request Oct 26, 2024
Merged
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xylar xylar xylar approved these changes

Assignees

@jonbob jonbob

Labels
mpas-ocean non-BFB PR makes roundoff changes to answers.
Projects
None yet
Milestone
v3.1alpha
Development

Successfully merging this pull request may close these issues.
5 participants
@cbegeman @rljacob @xylar @proteanplanet @jonbob