Sampling intermediate RK stages

siuwuncheung commented 3 years ago

Implement the capability of sampling intermediate RK stages during offline stage.

Command line option -sample-stages controls the activation of the new capability.
New member functions GetRKStages and GetRKTime are defined in the derived class HydroODESolver to enable the sampler access the RK stages and time stored in the time integrator.
In order to enable the capability, ode_solver has to belong to a derived class of HydroODESolver, e.g. RK2AvgSolver and RK4ROMSolver. The new capability is now only compatible with RK2 average and RK4 schemes.

dylan-copeland commented 3 years ago

@siuwuncheung After laghos.cpp:1537 which has samplerLast = NULL;, should there be if (rom_sample_stages) ode_solver_samp->SetSamplerLast(samplerLast); to set the last sampler to NULL in the ODE solver?

dylan-copeland commented 3 years ago

There are some failing regression tests, e.g. Verification failed: (rom_oper) is false: --> HydroSolvers expect ROMOperator. Also, some space-time regression tests fail, which I don't think is caused by this PR. The problem for those tests is Comparing: ROMsol/romS ROMsol exists on the baseline branch, but not on the user branch. triple-point-space-time-romhr: FAIL Maybe @kevinhkhuynh can help with the ROMsol issue.

chldkdtn commented 3 years ago

@dylan-copeland and @kevinhkhuynh , Let's review and approve this PR today if possible.

dylan-copeland commented 3 years ago

@siuwuncheung do the regression tests pass now?

kevinhkhuynh commented 3 years ago

There are some failing regression tests, e.g. Verification failed: (rom_oper) is false: --> HydroSolvers expect ROMOperator. Also, some space-time regression tests fail, which I don't think is caused by this PR. The problem for those tests is Comparing: ROMsol/romS ROMsol exists on the baseline branch, but not on the user branch. triple-point-space-time-romhr: FAIL Maybe @kevinhkhuynh can help with the ROMsol issue.

Are the space-time romhr regression tests working? I have run them manually and no romS files are outputted. Is that intended? If so, then I'll need to update the test script.

siuwuncheung commented 3 years ago

@dylan-copeland As @kevinhkhuynh has said, the space-time tests are failing. The ROMsol directory for both baseline and user branch in these tests are empty.

dylan-copeland commented 3 years ago

I confirmed that only the space-time regression tests are failing, and the reason is that the romS_* files are not written. These are ROM solutions written to file on each timestep in the spatial ROM case. For space-time, there is only one solution for all time, not solutions at each time. There is no restore phase for space-time, yet. When that is implemented, these files at each timestep could be written out to files for comparison.

I am not sure why this fails now but did not before. One solution would be to skip the comparison of these files in the space-time case, but that may be complicated for the regression testing scripts. @kevinhkhuynh do you think this would be easy? We can (i) let the tests fail for now and fix this later, (ii) skip this check for space-time tests (@kevinhkhuynh would need to do that), or (iii) implement a restore phase. For this PR, (i) seems most appropriate, unless (ii) can be done quickly.

chldkdtn commented 3 years ago

I confirmed that only the space-time regression tests are failing, and the reason is that the romS_* files are not written. These are ROM solutions written to file on each timestep in the spatial ROM case. For space-time, there is only one solution for all time, not solutions at each time. There is no restore phase for space-time, yet. When that is implemented, these files at each timestep could be written out to files for comparison.

I am not sure why this fails now but did not before. One solution would be to skip the comparison of these files in the space-time case, but that may be complicated for the regression testing scripts. @kevinhkhuynh do you think this would be easy? We can (i) let the tests fail for now and fix this later, (ii) skip this check for space-time tests (@kevinhkhuynh would need to do that), or (iii) implement a restore phase. For this PR, (i) seems most appropriate, unless (ii) can be done quickly.

@dylan-copeland I am okay if we take Option (ii).

kevinhkhuynh commented 3 years ago

I confirmed that only the space-time regression tests are failing, and the reason is that the romS_* files are not written. These are ROM solutions written to file on each timestep in the spatial ROM case. For space-time, there is only one solution for all time, not solutions at each time. There is no restore phase for space-time, yet. When that is implemented, these files at each timestep could be written out to files for comparison.

I am not sure why this fails now but did not before. One solution would be to skip the comparison of these files in the space-time case, but that may be complicated for the regression testing scripts. @kevinhkhuynh do you think this would be easy? We can (i) let the tests fail for now and fix this later, (ii) skip this check for space-time tests (@kevinhkhuynh would need to do that), or (iii) implement a restore phase. For this PR, (i) seems most appropriate, unless (ii) can be done quickly.

I just moved the online phase into the offline for now, meaning the online phase still runs, but only the offline stuff is checked.

chldkdtn commented 3 years ago

@siuwuncheung @dylan-copeland is this PR ready to merge?

siuwuncheung commented 3 years ago

@siuwuncheung @dylan-copeland is this PR ready to merge?

Now the PR gets all the approval and is ready to merge.

CEED / Laghos

Sampling intermediate RK stages #152