-
Notifications
You must be signed in to change notification settings - Fork 46
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
WIP: Rename regression_test.py to use pytest #279
base: master
Are you sure you want to change the base?
Conversation
Changes at around 1e-3 level probably due to different handling of refinement regions and resulting patches.
* master: (25 commits) Update some python code to remove errors Add *.data to gitignore Add basic CI script fix euler_3d_radial/qinit.f90 to agree with v4.x version create gauge filenames that allow more than 5 digits also in 1d check that abs(gaugeno) < 2**31 in data.py create gauge filenames that allow more than 5 digits also in 3d create gauge filenames that allow more than 5 digits in gauge number update examples/README.txt regarding env variables modify examples/README.txt with new instructions move run_examples.py to clawutil rename run_tests.py -> run_examples.py clean up run_tests.py fix typo in .gitignore cleaned up amrclaw files with SHARED do warnings new features: checkpt style 4 and added STOP feature bug fix for cases when only x is periodic Remove extra paren typo Drop Python 2 support, remove imports of six and __future__ Add meson.build. ...
The gauges disagree slightly at later times with old data. Running older versions of the code such as v5.8.0 give the same results as this new data, so not sure when it diverged. --------
Two tests are failing, @ketch or @mandli, could you try running |
I have the following with the full pytest output attached below.
|
I noticed that some of the tests were behaving oddly and made some adjustments to the adjoint tests. Unfortunately, while I think it fixed one problem, it caused another for the 1D version to surface. |
I tried running it on a linux machine and I got the same results as @mandli, and digging into the Maybe there's a cell with a value that's within rounding error of the flagging tolerance and if it gets flagged another row of cells is required in the patch. But strangely it's not just in one patch but in all 5 of the level 3 grids in the figure below. In one computation they all start at @mjberger is this something worth pursuing, or should we just change the tolerance a bit for example and hope it goes away? |
it's hugely annoying, sometimes impossible, to track down this kind of thing. How soon does it diverge into the calculation? Is it the same for enough time to be a meaningful test if you shorten the run time?
— Marsha
… On Jun 19, 2024, at 11:08 PM, Randall J. LeVeque ***@***.***> wrote:
I tried running it on a linux machine and I got the same results as @mandli <https://github.com/mandli>, and digging into the tests/acoustics_2d_radial results, the level 3 grids it makes are slightly different larger and hence the results start to diverge near the patch boundary. I thought it might be because I was using the ifort compiler, but redoing it with gfortran on linux didn't change the results (still different than with gfortran on my MacBook). In both tests I used a fresh clone of the repos plus this PR, so I think all the code should have been identical.
Maybe there's a cell with a value that's within rounding error of the flagging tolerance and if it gets flagged another row of cells is required in the patch.
But strangely it's not just in one patch but in all 5 of the level 3 grids in the figure below. In one computation they all start at y = -0.26 and in the other at y = -0.24.
@mjberger <https://github.com/mjberger> is this something worth pursuing, or should we just change the tolerance a bit for example and hope it goes away?
frame1.png (view on web) <https://github.com/clawpack/amrclaw/assets/720122/e2d0541e-0e44-4893-a103-60245a6ba26c>
—
Reply to this email directly, view it on GitHub <#279 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAGUGC6VM4X7YHUJ6RJMGLDZIJBSJAVCNFSM6AAAAABJEYUUPSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNZZG4ZDMMBUGU>.
You are receiving this because you were mentioned.
|
It's a small unit test problem and looking at |
Also updated regression data for 2 examples with minor changes in results.
See also