AMReX-Fluids / IAMR

A parallel, adaptive mesh refinement (AMR) code that solves the variable-density incompressible Navier-Stokes equations.
https://amrex-fluids.github.io/IAMR/
80 stars 57 forks source link

WIP: Update non eb gradp #80

Closed cgilet closed 3 years ago

cgilet commented 3 years ago
  1. Remove IAMR's getGradP() and always use MLMG to compute gradient(P).
  2. Add Gradp_Type to State.
  3. Also contains bug fixes for EB multilevel.
  4. Move predict_velocity() to NavierStokesBase. Since the PLM version was the same as NS, better to just have one function in NSB
emotheau commented 3 years ago

@cgilet I have tested with multiple combination of missing gradp or average data, and it seems to work. However, the safety net checking if data are present is overdoing its job. Usually we start a simulation with ns.avg_interval = 0 because we want to wait for a converged state before starting the process of averaging. When we turn ns.avg_interval = 1, it will create the Average_Type data that were missing from the previous checkpoint file. With the present version, the code will fail and say that data are missing (which makes sense, and the checking script works), and then we have to force the start of the simulation with ns.avg_in_checkpoint=0 to bypass the procedure.

The simple solution to fix this problem is to consider that a checkpoint file will always store Average_Type data, even if we don't actually do any averaging. The issue is that it will double the size of a checkpoint file for nothing. The other solution is to drop out the ns.avg_in_checkpoint check, assuming that it may be normal to have missing Average_Type data. What do you think about it?

emotheau commented 3 years ago

@cgilet Actually I just found that the restart with a missing GradP does not work with AMR. I get amrex::Error::0::get_data: invalid time !!! and the code crashes at line 4797 in the call MultiFab& Press = get_data(Press_Type, time);, from NavierStokesBase::computeGradP(Real time)

cgilet commented 3 years ago

@emotheau I think perhaps I chose a bad name for the variable ns.avg_in_checkpoint, and maybe I should rename it ns.avg_in_restart. It is meant only to specify whether the restart checkpoint file contains average data or not. It has nothing to do with what gets written to a new checkpoint file. I put this variable in because the restart code is not smart enough to tell/check which data are contained in the checkpoint, it only checks on the number of types, which creates the potential for reading Average_Type from the restart file and trying to put it in Gradp_Type. So, I think, for now at least, it is best to force users to specify exactly what is in the restart file. Please let me know if renaming the variable addresses your concern.

I will deal with the AMR restart issue tomorrow. I thought for sure I had tested it with multiple levels, sigh. Could you please put your restart file in a git branch for me to access?

emotheau commented 3 years ago

Ok everything seems fine now, the new GradP and Average feature are working with restart, AMR, and the conversion tool for checkpoint files. Thanks for this hard work !