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

Code review of hotstart code #51

Open
water-e opened this issue Dec 10, 2024 · 0 comments
Open

Code review of hotstart code #51

water-e opened this issue Dec 10, 2024 · 0 comments
Assignees

Comments

@water-e
Copy link
Contributor

water-e commented Dec 10, 2024

Please conduct a code review of the hotstart code?

Some important elements are:

  1. We need more comments or more function calls. The coding is too monolithic.
  2. Either hotstart or nudging has code that lists scalar names be abstracted out of the hotstart or nudging files and become its own capability. It is awfully nice to have and there is nothing particularly hotstarty/nudgy about it.
  3. Rename schism_hotstart.py as hotstart.py. Within the schimpy namespace there is no need to start all the file names with schism_*.
  4. It should work for island flooding. There have been indexing errors.
  5. It would be nice if we could maximize "exact" reuse of nodes. I'm not sure we know the extent to which grids built from maps with a lot of identical polygon specs and run through the preprocessor will be similar.
  6. A particular example of exact replication is the case when the hotstart is applied to the same grid and merely adds a variable.
  7. We should identify if/why turbulence or velocity causes problems.
  8. Examples in BayDeltaSCHISM should be brought up-to-date including schism_hotstart to hotstart and specification changes
  9. Visualization in the examples should be separated from generation. There seem to be lots of examples where the hotstart generates OK but the visualization code breaks.
  10. I'm a bit confused at times as to why some things are arguments and why others are in the yaml specification. Can we be sure there are good reasons?
  11. Should the examples be more annotated? They could refer back to the User Guide but also have most of the important switches explained in place?
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

No branches or pull requests

2 participants