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 HISTORY for coupled model applications #359

Merged
merged 12 commits into from
Sep 19, 2022

Conversation

sanAkel
Copy link
Collaborator

@sanAkel sanAkel commented Sep 14, 2022

This PR:

Thank you @sdrabenh for the above suggestions.

  • gcm_setup section for choice of HISTORY.rc, NOW clearly states what should be used when the ocean model == MOM5.

Both of the above HISTORY.rc files write monthly files and anything more frequent has been kept to the absolute least.

@sanAkel sanAkel added the 0 diff The changes in this pull request have verified to be zero-diff with the target branch. label Sep 14, 2022
@sanAkel sanAkel requested a review from mathomp4 September 14, 2022 21:52
@sanAkel sanAkel requested a review from a team as a code owner September 14, 2022 21:52
@sanAkel sanAkel self-assigned this Sep 14, 2022
@mathomp4 mathomp4 added the Contingent - DNA Do Not Approve (DNA). These changes are contingent on other PRs label Sep 15, 2022
@mathomp4
Copy link
Member

Putting a blocker on this until I can fix the bug in gcm_setup...plus I want to tweak it a bit anyway. :)

@sanAkel
Copy link
Collaborator Author

sanAkel commented Sep 15, 2022

I can fix the bug in gcm_setup...plus I want to tweak it a bit anyway. :)

@mathomp4 is there an issue that explains what the "bug" is about?

Copy link
Collaborator Author

@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.

I like these changes! Thank you @mathomp4

Copy link
Collaborator Author

@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.

Makes sense!

@sanAkel
Copy link
Collaborator Author

sanAkel commented Sep 15, 2022

I tested @mathomp4's addition(s): set up dummy exps (for MOM5 and MOM6), not with data ocean.

Works well, the gcm_setup ➡️ HISTORY bit is clear!

@mathomp4 mathomp4 removed the Contingent - DNA Do Not Approve (DNA). These changes are contingent on other PRs label Sep 15, 2022
@mathomp4
Copy link
Member

The CI is still unhappy. need to figure out why...

@mathomp4
Copy link
Member

The CI is still unhappy. need to figure out why...

Okay. I know why. I need to update the docker images because this change breaks my create_expt.py script without a few fixes. Time to fix it!

@mathomp4
Copy link
Member

Fingers crossed. I think this should work now.

@mathomp4
Copy link
Member

Okay. Now I think it should work. Feeling dumb today!

@mathomp4
Copy link
Member

Yay! Took a few tries, but it worked!

Copy link
Collaborator Author

@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.

Thanks for that indentation!

@sdrabenh sdrabenh merged commit e5766dd into develop Sep 19, 2022
@sdrabenh sdrabenh deleted the feature/sanAkel/coupled_Model-History-reorg branch September 19, 2022 20:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0 diff The changes in this pull request have verified to be zero-diff with the target branch.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants