Skip to content

Conversation

mandli
Copy link
Member

@mandli mandli commented Apr 22, 2025

A clean attempt at switch the tests over to PyTest. Also moves the tests out of a dedicated directory and aligns each with an example, modifying only what needs to be modified.

@mandli mandli self-assigned this Apr 22, 2025
@mandli mandli changed the title Switch to PyTest WIP: Switch to PyTest Apr 22, 2025
@mandli mandli marked this pull request as draft April 22, 2025 14:03
@rjleveque
Copy link
Member

rjleveque commented May 24, 2025

PR #649 suggests merging that one before this, but I think #649 has problems (see the discussion there), so perhaps the regression data needs to be updated in this PR?

@mandli
Copy link
Member Author

mandli commented May 25, 2025

Yeah, we can update the regression data when necessary.

@mandli
Copy link
Member Author

mandli commented Aug 1, 2025

I have done a review of all the tests and it appears that once a couple of bugs were fixed that the problems may now stem from the use of the modifications introduced in clawpack/riemann#183. Below are comparisons of the old test data and the new.

multilayer_4 netcdf_bowl particles_1 particles_2 vortex_0 vortex_1 vortex_2 vortex_3 vortex_4 bowl_slosh chile_adjoint dtopo multilayer_1 multilayer_2 multilayer_3

@mandli mandli changed the title WIP: Switch to PyTest Switch to PyTest Aug 7, 2025
@mandli mandli marked this pull request as ready for review August 7, 2025 13:34
@mandli
Copy link
Member Author

mandli commented Aug 7, 2025

I also should add that I added an example/test to address the recent changes to the Riemann solver. The test is in vortex and implements the moving vortex test case. It currently is simply a regression/validation test, but we could add a convergence test to it.

The more pressing matter though is where to put this example. I currently added a directory called shallow and stuck it in there as it has nothing to do with tsunamis or storm surge. There are other examples in tsunami that also are not explicitly tied to tsunamis, or moving topography. I am wondering what we would like to do about this. I know I have gotten some questions about where some examples are and people did not think to look in tsunami if they were not doing tsunamis.

@rjleveque
Copy link
Member

Regarding where to put examples that aren't a tsunami, surge, etc., we could just put them in the top level of the examples directory, e.g. in geoclaw/examples/vortex. The bowl-slosh example might belong there too. Things like bowl-radial and radial-ocean-island-fgmax are more ambiguous, since these were developed with tsunami modeling in mind but test propagation and inundation more generally.

@rjleveque
Copy link
Member

@mandli: regarding your comment

I have done a review of all the tests and it appears that once a couple of bugs were fixed that the problems may now stem from the use of the modifications introduced in clawpack/riemann#183. Below are comparisons of the old test data and the new.

I'm not sure I understand the plots. Several of them are labelled Gauge 1 and are very different from one another. Also one of them has a discontinuity in q[3,:] at t=0.5. What are these showing?

@mandli
Copy link
Member Author

mandli commented Aug 15, 2025

Regarding where to put examples that aren't a tsunami, surge, etc., we could just put them in the top level of the examples directory, e.g. in geoclaw/examples/vortex. The bowl-slosh example might belong there too. Things like bowl-radial and radial-ocean-island-fgmax are more ambiguous, since these were developed with tsunami modeling in mind but test propagation and inundation more generally.

Makes sense. Will change over and propose some examples to move into an upper directory.

@mandli
Copy link
Member Author

mandli commented Aug 15, 2025

@mandli: regarding your comment

I have done a review of all the tests and it appears that once a couple of bugs were fixed that the problems may now stem from the use of the modifications introduced in clawpack/riemann#183. Below are comparisons of the old test data and the new.

I'm not sure I understand the plots. Several of them are labelled Gauge 1 and are very different from one another. Also one of them has a discontinuity in q[3,:] at t=0.5. What are these showing?

Sorry, should have descriped the plots better, but I was being lazy and automated all the comparisons. The test in question is also not clearly labeled (it's in the legend). Here are some clarifications:

  1. The gauge number is from the test, some of them have multiple, such as the new vortex example.
  2. The left column is the gauge data plotted and the differences on the right if the size of the gauge output was the same.
  3. Each row is another field of the gauge (whatever was available, not necessarily what is tested against).

I am not sure about the discontinuity. That is the dtopo test, which was failing even before the changes, but I am not as familiar with that code nor test.

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

Successfully merging this pull request may close these issues.

2 participants