search

barbagroup / geoclaw

A fork of GeoClaw (http://www.clawpack.org/geoclaw) for hydrocarbon overland flow
BSD 3-Clause "New" or "Revised" License
3 stars 3 forks source link

Optimization and landspill-specific modification #17

Closed piyueh closed 5 years ago

piyueh commented 5 years ago

Major changes in this pull requests

1. Revert most of the original GeoClaw source files to v5.5.0

Before this PR, some source files under src/2d/shallow are modified to fit landspill module's need. This design causes some problems:

  1. When merging back to the upstream, it's prone to having conflicts because the upstream now has many new developments since we made the fork.

  2. The multi-layer module in the upstream code is neither entirely independent to the original GeoClaw nor completely integrated into GeoClaw. It uses its own Makefile to build a multi-layer solver. In a nutshell, things will be messy if we try to make the landspill module compatible with both original GeoClaw and multi-layer solvers.

So in this PR, we revert those already modified files back to their states in v5.5.0. The modified source files are now in the landspill module's folder. The way to use the landspill module is more like the way to use the multi-layer solver: users have to use Makefile.landspill, instead of Makefile.geoclaw.

This change also makes sure that all examples in other folders (tsunami, storm-surge, & multi-layer) can still run correctly.

It seems there's an effort to integrate the multi-layer module into the original GeoClaw solver. I think the full integration of the landspill module should only happen after the integration of the multi-layer module is done.

2. Add extra parameters into update.f90 and flag2refine2.f90

These changes are in the original update.f90 and flag2refine2.f90 under geoclaw/src/2d/shallow. In update.f90, users can set an extra parameter to control the minimum fluid surface level of a coarse cell when the averaged surface level of its child cells are below its topography elevation. In flag2refine2.f90, the added parameter controls the minimum fluid surface level that will trigger the refinement.

In order to let users set the two parameters, geoclaw/src/python/geoclaw/data.py is modified. The default values of the two parameters are the values used by the original GeoClaw.

3. Slight optimizations

  1. The stepgrid.f file in the folder landspill/optimized skips dry AMR patches. This change doesn't dramatically improve the performance because only the coarsest grid has entirely dry patches. But it still gives slight improvement. Users can compare the performances of the example utah_dem and utah_dem_opt, or utah_dem_hydro_feat and utah_dem_hydro_feat_opt.

  2. The flag2refine.f90 in the folder landspill/optimized excludes all refinement criteria not used by the landspill module and overland flows. This modification does not provide any performance improvement at all. But it gives a more unobstructed view of the code.

4. Move the data object LandSpillData to under the GeoClawData

Originally, LandSpillData is designed to be parallel with GeoClawData in the data structure, and this requires registering LandSpillData under the ClawRunData in clawutil package. Two concerns:

  1. This will make the process of merging the landspill fork back to GeoClaw annoying: we also have to make modifications to clawutil.
  2. This does not make sense because the landspill module is a module of GeoClaw. It's weird to register LandSpillData in ClawRunData and make LandSpillData in parallel with GeoClawData in the data structure.

So now the LandSpillData is just a member of the GeoClawData.

Minor changes/modifications

  1. Lat-long grids are also accepted if there's no point source.
  2. Fix the rounding error issue when identifying the cell indices of point sources. Due to floating numbers, if a point is located at the interface of two AMR grid patched, the old algorithm may not find the correct cell indices. I added some tolerance in the code to fix the issue.
  3. Adapt the landspill examples to changes introduced in this PR.
piyueh commented 5 years ago

Hi @mandli, this pull request is ready for review. I added an explanation to the pull request. I appreciate your help with reviewing out PRs!

mandli commented 5 years ago

Will do!

piyueh commented 5 years ago

Thank you @mandli ! If you would like, we can discuss more regarding the compatibility.

mandli commented 5 years ago

Sounds good but we can take that offline for the moment.