Skip to content

Conversation

@alperaltuntas
Copy link
Member

This PR:

  • Removes duplicate entries in MOM_input and input_data_list
  • Adds a CI test to detect future duplicate entries.
  • Fixes restart file check for multi-instance, fresh hybrid and branch runs.

Testing: aux_mom.derecho

@mnlevy1981
Copy link
Collaborator

I did a little poking around, and it seems like duplicate entries in a YAML file violate the spec, but pyyaml doesn't throw an error for that. I'm a little surprised it's not even an option in safe_load(), because that seems much easier than writing a bespoke test... but I'll block off some time tomorrow to take a closer look at this PR

@alperaltuntas
Copy link
Member Author

@mnlevy1981 FYI, the tests/check_duplicate_params.py was a bit of “vibe coding,” but given the short timeline and the fact that it’s not user-facing, I thought having some tests was better than none.

Copy link
Collaborator

@mnlevy1981 mnlevy1981 left a comment

Choose a reason for hiding this comment

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

The cleanup of the YAML files (and generated json files) is straight-forward, and I like the addition of this test to github actions. I didn't do an in-depth review of check_duplicate_params.py, but I did play around with it in a local checkout and verified it returned an error when the yaml file had duplicate keys and passed when it did not. I'm still surprised this sort of test isn't available in pyyaml, but maybe someday pyyaml will abide by the YAML spec and we can get rid of this test.

The cleanup to finding restart files in buildnml seems orthogonal to the stated goal of this PR, but is useful for the upcoming tags so we should keep it in.

@alperaltuntas alperaltuntas merged commit 1e897ce into ESCOMP:main Oct 23, 2025
6 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.

2 participants