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

Update branch, add testing workflow, use implicit none in more Riemann solvers #4

Merged
merged 25 commits into from
Sep 19, 2024

Conversation

carlosmunozmoncayo
Copy link

@carlosmunozmoncayo carlosmunozmoncayo commented Sep 15, 2024

With these changes, there are no longer variables with implicit data type in the Riemann solvers. For the moment, the testing workflow is using PyClaw's tests.

YuchengZhang-hub and others added 20 commits April 19, 2022 14:33
aux1,aux2,aux3 , wrong variable size declaration under rpt2_single_layer
These were accidentally added in my last commit.
Remove lines related to experimental 1D geoclaw Riemann solver.
Fix small but confusing typo in instructions for adding Riemann solvers.
Lagrangian gas dynamics with spatially-varying
(but temporally constant) entropy.
Add Riemann solver for p-system.
Update rpt2_layered_shallow_water.f90
Remove the py_dep dependency from extension modules in riemann/meson.build
update rpn2_geoclaw, geoclaw_riemann_utils
* Build 3d Euler with gravity at installation

* Fix typo

* Add imports to __init__.py
It uses PyClaw's tests since, to my understanding, an ensemble
of local regression tests for the Riemann repository using
Pytest would require the appropriate argument intents of the
rp* subroutines to be explicitly declared already.
So the Riemann solvers should be modernized first.

Also, the test for shallow water on the sphere is omitted for the moment.
@carlosmunozmoncayo carlosmunozmoncayo changed the title Update branch with changes in master and add a temporary testing workflow using PyClaw's tests Update branch, add testing workflow, use implicit none in more Riemann solvers Sep 15, 2024
The PyClaw test that uses this RS is skipped in the testing
workflow since it leads to a core dump
(I suspect this has to do with the OS or the system
architecture).

However, this test passes locally for me (before and after the
implicit none changes).
The regression tests in AMRClaw are not being collected by Pytest. They
just have to be renamed. This is a temporary fix to run them from this
repo in a single pytest session.
@carlosmunozmoncayo
Copy link
Author

The behavior of one Riemann solver seems to have changed. The regression tests in clawpack/amrclaw/tests/acoustics_2d_adjoint are failing in the CI workflow and locally for me. Please do not merge this yet, I'll try to find out what happened.

@ketch
Copy link
Owner

ketch commented Sep 19, 2024

That's very strange, since it's hard to see what is different about that example. I took a quick look but didn't spot anything that seemed likely to cause a problem.

@carlosmunozmoncayo
Copy link
Author

The same test fails if I use the Riemann solvers without the modifications in this PR. The difference between the expected and reference solutions is also the same. I think that the behavior of the Riemann solver was not affected.

@ketch
Copy link
Owner

ketch commented Sep 19, 2024

Okay. In that case I will merge this and open a PR to the master branch.

@ketch ketch merged commit 2786fc4 into ketch:implicit_none Sep 19, 2024
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

Successfully merging this pull request may close these issues.

8 participants