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

Set default time_integrator for Omega #232

Merged
merged 2 commits into from
Oct 15, 2024

Conversation

xylar
Copy link
Collaborator

@xylar xylar commented Sep 30, 2024

In the manufactured_solution test, this needs to be RungeKutte4. (I'm not worrying about other tests yet.)

Checklist

  • Testing comment in the PR documents testing used to verify the changes

@xylar xylar added bug Something isn't working ocean Related to ocean tests or analysis labels Sep 30, 2024
@xylar xylar self-assigned this Sep 30, 2024
@xylar
Copy link
Collaborator Author

xylar commented Sep 30, 2024

This PR addresses the error found in E3SM-Project/Omega#137.

@xylar
Copy link
Collaborator Author

xylar commented Sep 30, 2024

Testing

I am able to run the 200km forward run from manufactured_solution with this change on Chrysalis, whereas it failed without this change. (The check for output.nc then fails as expected.)

@xylar
Copy link
Collaborator Author

xylar commented Sep 30, 2024

@hyungyukang, would you be willing to review this fix as well? Perhaps you can test along with other testing you're doing anyway.

@hyungyukang
Copy link

@hyungyukang, would you be willing to review this fix as well? Perhaps you can test along with other testing you're doing anyway.

Yes. I will test this PR as well!

In the manufactured_solution test, this needs to be RungeKutte4
Copy link

@hyungyukang hyungyukang left a comment

Choose a reason for hiding this comment

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

@xylar , Thanks for updating this. I was able to create the manufactured solution test with Omega model (--model=omega). In omega.yml in the test directory, the TimeStepper is correctly configured as RungeKutta4.

I'm approving this PR based on my testing and @xylar 's as well as visual inspection. Thanks @xylar !

@xylar
Copy link
Collaborator Author

xylar commented Oct 15, 2024

Thanks, @hyungyukang!

@xylar xylar merged commit ece864c into E3SM-Project:main Oct 15, 2024
5 checks passed
@xylar xylar deleted the fix-omega-time-integrator branch October 15, 2024 00:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working ocean Related to ocean tests or analysis
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants