AMReX-Astro / AMReX-Astro/Castro

Merge construct_hydro_source for MOL and CTU

Open
#522 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

clean-up hydro
Dominant language
C++
Stars
340
Forks
105
Avg merge
3d 8h
Merged PRs (30d)
8

Description

We should be able to merge the hydro source constructor for both MOL and CTU. If we pass dt = 0 to the CTU hydro source constructor, the code should already do the right thing for this case, which is how MOL calls it. (If it doesn't, we'll have to fix that.) Then the only thing to be concerned about is optimization -- the trans* calls are unnecessary if dt = 0. So there would be a switch that says if dt > 0, then take the normal CTU route, and if dt == 0, then pass qm and qp directly to cmpflx.

The reason to do this is that we don't have to maintain two sections of nearly redundant code. Aside from the calls to the Riemann solver (and the trans terms), the MOL hydro update is very similar.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Locate the MOL and CTU hydro source constructors and trace their calls to trans* and cmpflx. Check whether the CTU constructor already handles dt = 0 as described, then compare the two update paths. Done means the constructors share one implementation while preserving the dt > 0 path and avoiding unnecessary trans* calls for dt == 0.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
hpc, performance
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
30/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.