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 esm4p2 and null model yamls for c5 update #270

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

rem1776
Copy link
Contributor

@rem1776 rem1776 commented Nov 15, 2024

Describe your changes

just updates the esm4p2 and null model example yamls to work with the latest c5 modules

Checklist before requesting a review

  • I ran my code
  • I tried to make my code readable
  • I tried to comment my code
  • I wrote a new test, if applicable
  • I wrote new instructions/documentation, if applicable
  • I ran pytest and inspected it's output
  • I ran pylint and attempted to implement some of it's feedback

Copy link
Collaborator

@singhd789 singhd789 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 updating this Ryan! I just have a few clarifying questions. Also, do you think we should keep JUST the null model example in here for testing, and move the AM5 and ESM4p2 completely over to fre-examples? I think this would make it slightly less convoluted here.

fre/make/tests/ESM4_example/compile.yaml Outdated Show resolved Hide resolved
fre/make/tests/ESM4_example/platforms.yaml Show resolved Hide resolved
@rem1776
Copy link
Contributor Author

rem1776 commented Nov 15, 2024

Thanks for updating this Ryan! I just have a few clarifying questions. Also, do you think we should keep JUST the null model example in here for testing, and move the AM5 and ESM4p2 completely over to fre-examples? I think this would make it slightly less convoluted here.

Yeah I think thats a great idea, I'll remove the directories with this PR and make a new one to fre-examples.

@rem1776
Copy link
Contributor Author

rem1776 commented Nov 15, 2024

@singhd789 I think a couple tests use the am5.yaml so this will break the testing. Maybe we should keep those around for now, unless we want to go crazy and add another git submodule for the fre-examples repo lol.

@singhd789
Copy link
Collaborator

singhd789 commented Nov 15, 2024

@singhd789 I think a couple tests use the am5.yaml so this will break the testing. Maybe we should keep those around for now, unless we want to go crazy and add another git submodule for the fre-examples repo lol.

Oh interesting. I think you may be right (fre cli make tests maybe). Now I'm wondering how the checks passed without those. We can keep the AM5 yamls in for now if needed (we should change those to use the null model). Good catch!

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.

2 participants