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

We need more checks to avoid simple errors in setting up a problem #737

Open
ketch opened this issue Dec 1, 2024 · 2 comments
Open

We need more checks to avoid simple errors in setting up a problem #737

ketch opened this issue Dec 1, 2024 · 2 comments

Comments

@ketch
Copy link
Member

ketch commented Dec 1, 2024

Recently I ran across a couple of things that we don't check and which I wasted significant time debugging in my own scripts:

  • When using an f-wave solver, we don't check that solver.fwave==True. Running without setting this option will lead to nonsensical results (and often blowup, eventually).
  • We don't check that the aux array has been initialized. For instance, in shallow water with bathymetry, you can start a simulation without setting up the bathymetry (usually leading immediately to NaNs).
    We should check for these things when Controller.run() is called.
@mandli
Copy link
Member

mandli commented Dec 1, 2024

First idea sounds good to me. Maybe a broader consistency check but we kind of do that already I suppose with num_eqn, etc.

The aux arrays may not be able to be filled in until after run is called although we could initialize the aux array to something temporarily. I might hesitate to do this as reasonable initial values may lead to more difficulty when debugging as NaNs would be easier to spot.

@ketch
Copy link
Member Author

ketch commented Dec 2, 2024

I guess you are right about the second one. Since we use a setter function for the aux array, we could add a flag that gets set the first time the setter is called. But that's a potential performance hit every time aux is modified, just to avoid a rather rare stupid mistake, so probably not worth it.

I've opened PRs that handle the fwave issue.

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