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

Need upwind advection test -> Test coverage and Options Implementation #140

Closed
apcraig opened this issue May 11, 2018 · 2 comments
Closed
Assignees
Labels

Comments

@apcraig
Copy link
Contributor

apcraig commented May 11, 2018

In the test matrix,

https://docs.google.com/spreadsheets/d/1dMzOC2GAsGqXJvs7xHybCQkPRneeUm78Ah1OXp34bAQ

alt04 is supposed to test the upwind advection scheme but this causes the model to fail so remap is being used temporarily. We need to add or modify one of the tests to cover upwind.

@apcraig apcraig changed the title Need upwind advection test Need upwind advection test -> Test coverage and Options Implementation Nov 6, 2018
@apcraig
Copy link
Contributor Author

apcraig commented Nov 6, 2018

I am going to add a couple other issues here for the time being.

The alt01 test has bathymetry turned on but kdyn=0 (dynamics off). We should again review our tests and test coverage.

There is also a question about how to handle the options (--sets). There is a question of precedent and some problems have arisen in usage. At this point, there are few constraints on what can be defined in the set_nml and set_env options. Users can mix and match multiple options and the changes will be implemented based on the option name alphabetically. This can create problems if conflicting settings are defined in different options files and a user chooses them. This probably needs to be fixed. There are a couple ways this could be fixed.

  1. We could carefully review the options files and split up any files that might be in conflict. That would mean we would probably add more options files and tests would need more options to be fully defined.

  2. We could create a hierarchy of changes such that each set_nml or set_env file would define a value for a unique variable or group of variables. Then a higher level set of files would aggregate those. For instance, we might have set_nml.kdyn0, set_nml.kdyn1, set_nml.kdyn2, set_nml.kdyn3, ... and then a separate like set_nml.alt02 that would define it's options from the lower level files. That would allow just about any test to be setup from the low level files cleanly (with lots of options) and for aggregated settings to be defined as well. The original implementation of the options did something like this.

  3. We could allow just a single option to be defined by the user on the --sets argument so any conflicts would be avoided. That would mean we would have to create enough high level groups of settings to be useful, and it would reduce some of the flexbility of the current capability.

There may be other ways to address this issue as well.

@eclare108213
Copy link
Contributor

I will move the discussion about how to handle options to a separate issue, and close this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants