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

WIP: Allow specifying which components of q to output on each fgout grid #17

Merged
merged 15 commits into from
Aug 21, 2024

Conversation

rjleveque
Copy link
Collaborator

This requires clawpack/geoclaw#624

The main Makefile.dclaw now uses fgout_module.f90 from geoclaw rather than a separate one for D-Claw.

(I also added a couple directories tests/geoclaw_radial_slide and tests/geoclaw_bouss_radial_slide for testing geoclaw SWE and Bouss on this radial problem with water on the hill instead of dirt, perhaps useful to others, but these don't have fgout grids.)

I updated the example in examples/radial_slide to add

fgout.q_out_vars = [1,4,8]

to output h, hm, and eta at 100 fgout frames.

There's also a new pyvista_plotting.py module to make a 3D plot using PyVista, with the surface colored by mass fraction (hm / h).

A few frames shown below:
PyVistaFrame0001
PyVistaFrame0033
PyVistaFrame0078

* origin/main:
  minor change: move g=grav above loop on cells in rpt2
  set g=grav in transverse solver
* origin/main:
  add momentum autostop for binary
  delete fgmax files (address this later)
  this ran and wrote a file with the right number of columns.
  set tau to zero for very low m.
  improve filpatch.f90 and filval.f90 to fix m0 for refinement.
  adding fixed grid routines to dclaw to accommodate meqn=7.
  define g as dave had on main
  define g if bednormal is not 1
  d0s
  subtract sea level
Restored geoclaw_radial_slide/set*.py and included fgout specification in
radial_slide/setrun.py for further testing.

* origin/main:
  put output time back to 3.
  update radial slide setrun
  add more indicies to qinit
  fix index error from first pass
  first pass at moving to indicies.
  remove old
  move bowl slosh from examples to test
  starting to remove examples (except for radial slide)
case for adding Bouss terms.  This version runs GeoClaw, not D-Claw.
Note that with order=2 there is some oscillation in the leading shock,
more pronounced with transverse_waves = 1 than with 2.

#list of common modules needed for dclaw codes
COMMON_MODULES += \
$(DIGLIB)/digclaw_module.f90 \
$(DIGLIB)/qinit_module.f90 \
$(DIGLIB)/auxinit_module.f90 \
$(DIGLIB)/fgout_module.f90 \
Copy link
Collaborator

Choose a reason for hiding this comment

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

@rjleveque , @dlgeorge I suggest we delete dclaw/src/2d/dig/fgout_module.f90.

We only have it it as a placeholder that is no longer needed b/c Randy updated the GEOLIB/fgout_module.f90.

Thoughts?

Copy link
Owner

Choose a reason for hiding this comment

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

seems best to remove it I guess.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think that's fine. The only difference in it was that it set

fg%num_vars(1) = 10 ! DIG different number

but this is no longer needed with the geoclaw updates.

@kbarnhart
Copy link
Collaborator

@rjleveque I've tested this and it works well for me.

My only thoughts are

(1) that we should remove the two tests because they are not really of dclaw. Should these not go in the geoclaw/tests/geoclaw_radial_slide and geoclaw/tests/geoclaw_bouss_radial_slide?

(2) that we remove the dclaw/src/2d/dig/fgout_module.f90 file.

Let me know what you think (also tagging @dlgeorge).

Other than these two issues, I think this is ready to merge.

@kbarnhart kbarnhart merged commit dda4bc9 into geoflows:main Aug 21, 2024
@dlgeorge
Copy link
Collaborator

@rjleveque @kbarnhart . plots look great, and q_out_vars method seems good to me. PR seems good to me (?).

@rjleveque
Copy link
Collaborator Author

@kbarnhart: regarding moving the examples to geoclaw, I put them here since I was using the SWE version to compare to what I got when running D-Claw with mass fraction 0, so that it was pure water. I don't think it really makes sense to include in GeoClaw since it's not a situation most GeoClaw users would be setting up, although I suppose it could be interesting as a radially symmetric overland flood example.

At any rate I'm fine with removing them from the tests/ directory since normally we use this directory for unit tests (and hopefully we'll get continuous integration working again soon so these are used more regularly). Maybe they should be in a new subdirectory dev/ to keep them around for use in further developing the code? Or I'm fine with removing them from the repo and I'll just keep them locally.

(The geoclaw_bouss_radial_slide was committed by accident since we don't have bouss in D-Claw yet for comparison.)

@kbarnhart
Copy link
Collaborator

@rjleveque : @dlgeorge and I discussed and we decided that it made sense to keep them here, broadly for the reasons you list. I think keeping geoclaw_bouss_radial_slide makes sense (eventually) when dclaw-bouss exists.

So I went ahead and merged it. Thanks for this PR!

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.

4 participants