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 run time information in NUOPC cap #1608

Merged

Conversation

jiandewang
Copy link
Collaborator

this mini PR added some run time information in NUOPC cap for us to monitor the UFS running speed. A logic variable "write_runtimelog" is added here with default value of False. As usual I am asking MOM6 major users for the review although only NCAR may use this feature (and we asked @alperaltuntas and @marshallward for a pre-review before this formal PR). Thanks for your very helpful suggestions.

jiandewang and others added 4 commits May 22, 2023 14:40
…n-20230504

update to MOM6 main 20230504 updating
…n-20230811

update to MOM6 main 20230731 and 20230811 updating
* add optional run time info in nuopc cap. Author: Jun Wang
@codecov
Copy link

codecov bot commented Sep 8, 2023

Codecov Report

Merging #1608 (f384a23) into main (2dd99f8) will increase coverage by 0.00%.
The diff coverage is n/a.

❗ Current head f384a23 differs from pull request most recent head be40a41. Consider uploading reports for the commit be40a41 to get more accurate results

@@           Coverage Diff           @@
##             main    #1608   +/-   ##
=======================================
  Coverage   38.17%   38.17%           
=======================================
  Files         269      269           
  Lines       76503    76503           
  Branches    14064    14064           
=======================================
+ Hits        29202    29204    +2     
+ Misses      42054    42052    -2     
  Partials     5247     5247           

see 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@sanAkel sanAkel left a comment

Choose a reason for hiding this comment

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

Changes are in the NUOPC driver that is not used in GEOS application.

Copy link
Collaborator

@marshallward marshallward left a comment

Choose a reason for hiding this comment

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

I am approving this, but I would like to repeat my concern about using MPI_Wtime in this driver. The function can become blocking if certain flags are set, which will block across the entire MPI_COMM_WORLD, not just MOM6. If other ranks are not calling MPI_Wtime, then the model will hang.

We recognize that your groups are maintaining the NUOPC cap, so I won't let this concern hold up the patch. But if a future NUOPC user encounters problems with this, then we may want to revisit this change.

Copy link
Collaborator

@abozec abozec left a comment

Choose a reason for hiding this comment

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

We don't use the NUOPC cap in our applications, so it won't affect us.

Copy link
Collaborator

@kshedstrom kshedstrom left a comment

Choose a reason for hiding this comment

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

This code doesn't affect me either.

@jiandewang
Copy link
Collaborator Author

thanks for everybody's time and effect, I am going to merge it

@jiandewang jiandewang merged commit afb3a44 into mom-ocean:main Sep 13, 2023
10 checks passed
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.

7 participants