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 and improve Riemann solver code #176

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
66 changes: 66 additions & 0 deletions .github/workflows/amrclaw_tests.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
name: Test Riemann solvers with AMRClaw tests

on:
push:
branches: [ "implicit_none" ]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This won't work in general and is specific to only this branch/PR

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes, that was just a hack to get it to test this branch for the moment.

pull_request:
branches: [ "implicit_none" ]

workflow_dispatch:

permissions:
contents: read

env:
CLAW: ${{ github.workspace }}

jobs:
build:
runs-on: ubuntu-latest
steps:
- name: Set up Python 3.10
uses: actions/setup-python@v5
with:
python-version: "3.10"

- name: Install dependencies
run: |
sudo apt-get update
sudo apt-get install gfortran

python -m pip install --upgrade pip
pip install 'numpy<2.0'
pip install matplotlib #Some imports require matplotlib
pip install scipy #To not skip tests
pip install nose
pip install flake8 meson-python ninja pytest

- name: Checkout Clawpack
uses: actions/checkout@v4.1.5
with:
repository: clawpack/clawpack
submodules: true

- name: Checkout AMRClaw branch
uses: actions/checkout@v4.1.5
with:
repository: clawpack/amrclaw #
path: amrclaw
ref: master

- name: Checkout implicit_none from this repo
uses: actions/checkout@v4.1.5
with:
repository: ${{ github.repository }}
path: ${{ env.CLAW }}/riemann
ref: implicit_none

- name: Install Clawpack
run: |
cd ${CLAW}
pip install --no-build-isolation --editable .

- name: Test with pytest
run: |
cd ${CLAW}/riemann
bash collect_pytests_amrclaw.sh
66 changes: 66 additions & 0 deletions .github/workflows/classic_tests.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
name: Test Riemann solvers with Classic Clawpack tests

on:
push:
branches: [ "implicit_none" ]
pull_request:
branches: [ "implicit_none" ]

workflow_dispatch:

permissions:
contents: read

env:
CLAW: ${{ github.workspace }}

jobs:
build:
runs-on: ubuntu-latest
steps:
- name: Set up Python 3.10
uses: actions/setup-python@v5
with:
python-version: "3.10"

- name: Install dependencies
run: |
sudo apt-get update
sudo apt-get install gfortran

python -m pip install --upgrade pip
pip install 'numpy<2.0'
pip install matplotlib #Some imports require matplotlib
pip install scipy #To not skip tests
pip install nose
pip install flake8 meson-python ninja pytest

- name: Checkout Clawpack
uses: actions/checkout@v4.1.5
with:
repository: clawpack/clawpack
submodules: true

- name: Checkout Classic branch
uses: actions/checkout@v4.1.5
with:
repository: clawpack/classic #
path: classic
ref: master

- name: Checkout implicit_none from this repo
uses: actions/checkout@v4.1.5
with:
repository: ${{ github.repository }}
path: ${{ env.CLAW }}/riemann
ref: implicit_none

- name: Install Clawpack
run: |
cd ${CLAW}
pip install --no-build-isolation --editable .

- name: Test with pytest
run: |
cd ${CLAW}/classic
pytest tests
65 changes: 65 additions & 0 deletions .github/workflows/pyclaw_tests.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
name: Test Riemann solvers with PyClaw tests

on:
push:
branches: [ "implicit_none" ]
pull_request:
branches: [ "implicit_none" ]

workflow_dispatch:

permissions:
contents: read

env:
CLAW: ${{ github.workspace }}

jobs:
build:
runs-on: ubuntu-latest
steps:
- name: Set up Python 3.10
uses: actions/setup-python@v5
with:
python-version: "3.10"

- name: Install dependencies
run: |
sudo apt-get update
sudo apt-get install gfortran

python -m pip install --upgrade pip
pip install 'numpy<2.0'
pip install matplotlib #Some imports require matplotlib
pip install scipy #To not skip tests
pip install flake8 meson-python ninja pytest

- name: Checkout Clawpack
uses: actions/checkout@v4.1.5
with:
repository: clawpack/clawpack
submodules: true

- name: Checkout PyClaw branch
uses: actions/checkout@v4.1.5
with:
repository: clawpack/pyclaw #
path: pyclaw
ref: master

- name: Checkout implicit_none from this repo
uses: actions/checkout@v4.1.5
with:
repository: ${{ github.repository }}
path: ${{ env.CLAW }}/riemann
ref: implicit_none

- name: Install Clawpack
run: |
cd ${CLAW}
pip install --no-build-isolation --editable .

- name: Test with pytest
run: |
cd ${CLAW}/pyclaw
pytest --ignore=development --ignore=examples/shallow_sphere
19 changes: 19 additions & 0 deletions collect_pytests_amrclaw.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
#Collect Pytest tests for AMRClaw
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am confused by this script. Does the workflow above for AMRClaw not work?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I try pytest ${CLAW}/amrclaw, Pytest does not collect the regression tests in AMRClaw. I think it tries to match the pattern *_test.py and skips regression_tests.py. This script was just a temporary way to collect the tests and pass them explicitly to Pytest. Probably the correct thing to do would be to rename the tests in the AMRClaw repo and not merge this script. I can open a PR there for that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Modifications were required to get the collection to work as you mentioned. This is hopefully going to fix that. Not sure what to do until then.

cd ${CLAW}/amrclaw/tests
pathlist=()
for dir in $(ls -d */); do
cd $dir
#Check if there are any test files
#Capture the error message and check if it is empty
if [ -z "$(ls *test*.py 2>/dev/null )" ]; then
cd ..
continue
fi
for file in $(ls *test*.py); do
pathlist+=($(pwd)/$file)
done
cd ..
done

#Run the tests
pytest ${pathlist[@]}
26 changes: 13 additions & 13 deletions src/rp1_acoustics.f90
Original file line number Diff line number Diff line change
Expand Up @@ -29,23 +29,23 @@ subroutine rp1(maxm,meqn,mwaves,maux,mbc,mx,ql,qr,auxl,auxr,wave,s,amdq,apdq)
! From the basic clawpack routines, this routine is called with ql = qr


implicit double precision (a-h,o-z)
implicit none

dimension wave(meqn, mwaves, 1-mbc:maxm+mbc)
dimension s(mwaves,1-mbc:maxm+mbc)
dimension ql(meqn, 1-mbc:maxm+mbc)
dimension qr(meqn, 1-mbc:maxm+mbc)
dimension apdq(meqn, 1-mbc:maxm+mbc)
dimension amdq(meqn, 1-mbc:maxm+mbc)
integer, intent(in) :: maxm, meqn, mwaves, mbc, mx, maux
real(kind=8), intent(in) :: ql(meqn, 1-mbc:maxm+mbc),qr(meqn, 1-mbc:maxm+mbc)

! local arrays
! ------------
dimension delta(2)
real(kind=8), intent(out) :: wave(meqn, mwaves, 1-mbc:maxm+mbc)
real(kind=8), intent(out) :: s(mwaves,1-mbc:maxm+mbc)
real(kind=8), intent(out) :: apdq(meqn, 1-mbc:maxm+mbc), amdq(meqn, 1-mbc:maxm+mbc)

! # density, bulk modulus, and sound speed, and impedence of medium:
! # (should be set in setprob.f)
common /cparam/ rho,bulk,cc,zz
real(kind=8) :: delta(2)
real(kind=8) :: rho, bulk, cc, zz
real(kind=8) :: a1, a2, auxl, auxr
integer :: i, m

! density, bulk modulus, and sound speed, and impedence of medium:
! (should be set in setprob.f)
common /cparam/ rho,bulk,cc,zz

! # split the jump in q at each interface into waves

Expand Down
34 changes: 20 additions & 14 deletions src/rp1_acoustics_adjoint.f90
Original file line number Diff line number Diff line change
Expand Up @@ -25,20 +25,26 @@ subroutine rp1(maxm,meqn,mwaves,maux,mbc,mx,ql,qr,auxl,auxr,fwave,s,amdq,apdq)

! # aux arrays not used in this solver.

implicit double precision (a-h,o-z)
implicit none

dimension fwave(meqn, mwaves, 1-mbc:maxm+mbc)
dimension s(mwaves, 1-mbc:maxm+mbc)
dimension ql(meqn, 1-mbc:maxm+mbc)
dimension qr(meqn, 1-mbc:maxm+mbc)
dimension apdq(meqn, 1-mbc:maxm+mbc)
dimension amdq(meqn, 1-mbc:maxm+mbc)
dimension auxl(maux, 1-mbc:maxm+mbc)
dimension auxr(maux, 1-mbc:maxm+mbc)
!Input
integer, intent(in) :: maxm, meqn, mwaves, maux, mbc, mx
double precision, intent(in) :: ql(meqn, 1-mbc:maxm+mbc)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might suggest that we consistently use real(kind=8) rather than double precision or if we are being really crazy implement the ability to have variable precision with something like real(kind=RPP).

double precision, intent(in) :: qr(meqn, 1-mbc:maxm+mbc)
double precision, intent(in) :: auxl(maux, 1-mbc:maxm+mbc)
double precision, intent(in) :: auxr(maux, 1-mbc:maxm+mbc)

double precision, intent(out) :: fwave(meqn, mwaves, 1-mbc:maxm+mbc)
double precision, intent(out) :: s(mwaves, 1-mbc:maxm+mbc)
double precision, intent(out) :: amdq(meqn, 1-mbc:maxm+mbc)
double precision, intent(out) :: apdq(meqn, 1-mbc:maxm+mbc)

!local
integer :: i, m
double precision :: rhoi, bulki, beta1, beta2
double precision :: rho, bulk, cc, zz
double precision, dimension(2) :: delta

! local arrays
! ------------
dimension delta(2)

! # density, bulk modulus, and sound speed, and impedence of medium:
! # (should be set in setprob.f)
Expand All @@ -47,7 +53,7 @@ subroutine rp1(maxm,meqn,mwaves,maux,mbc,mx,ql,qr,auxl,auxr,fwave,s,amdq,apdq)
! # split the jump in f(q) at each interface into waves

! # find b1 and b2, the coefficients of the 2 eigenvectors:
do 20 i = 2-mbc, mx+mbc
do i = 2-mbc, mx+mbc
! # material properties
rhoi = zz/cc
bulki = rhoi*cc**2
Expand All @@ -70,7 +76,7 @@ subroutine rp1(maxm,meqn,mwaves,maux,mbc,mx,ql,qr,auxl,auxr,fwave,s,amdq,apdq)
fwave(2,2,i) = -beta2*zz
s(2,i) = cc

20 END DO
END DO
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor niggle - code is yelling.



! # compute the leftgoing and rightgoing flux differences:
Expand Down
29 changes: 15 additions & 14 deletions src/rp1_acoustics_variable.f90
Original file line number Diff line number Diff line change
Expand Up @@ -36,20 +36,21 @@ subroutine rp1(maxm,meqn,mwaves,maux,mbc,mx,ql,qr,auxl,auxr,wave,s,amdq,apdq)
! From the basic clawpack routines, this routine is called with ql = qr


implicit double precision (a-h,o-z)

dimension auxl(maux, 1-mbc:maxm+mbc)
dimension auxr(maux, 1-mbc:maxm+mbc)
dimension wave(meqn, mwaves, 1-mbc:maxm+mbc)
dimension s(mwaves, 1-mbc:maxm+mbc)
dimension ql(meqn, 1-mbc:maxm+mbc)
dimension qr(meqn, 1-mbc:maxm+mbc)
dimension apdq(meqn, 1-mbc:maxm+mbc)
dimension amdq(meqn, 1-mbc:maxm+mbc)

! local arrays
! ------------
dimension delta(2)
implicit none

integer, intent(in) :: maxm, meqn, mwaves, maux, mbc, mx
real(kind=8), intent(in) :: ql(meqn, 1-mbc:mx+mbc)
real(kind=8), intent(in) :: qr(meqn, 1-mbc:mx+mbc)
real(kind=8), intent(in) :: auxl(maux, 1-mbc:maxm+mbc)
real(kind=8), intent(in) :: auxr(maux, 1-mbc:maxm+mbc)
real(kind=8), intent(out) :: wave(meqn, mwaves, 1-mbc:maxm+mbc)
real(kind=8), intent(out) :: s(mwaves, 1-mbc:maxm+mbc)
real(kind=8), intent(out) :: apdq(meqn, 1-mbc:maxm+mbc)
real(kind=8), intent(out) :: amdq(meqn, 1-mbc:maxm+mbc)

real(kind=8) :: delta(2)
real(kind=8) :: zi, zim, a1, a2
integer :: i, m


! split the jump in q at each interface into waves
Expand Down
Loading