MFC icon indicating copy to clipboard operation
MFC copied to clipboard

Equation of state is not DRY

Open sbryngelson opened this issue 3 years ago • 3 comments

The equation of state is used to compute the pressure from the variables. This EOS changes slightly for different models. The EOS implementation is repeated several places in the code, including in m_variables_converison, m_data_output (including serial data output and probe outputs), and likely more. Following the DRY principle (don't repeat yourself), we should fix this. It has also caused multiple false bugs in the past.

sbryngelson avatar Nov 12 '21 05:11 sbryngelson

This above doesn't even mention the fact that this same problem is repeated in the pre-process and post-process codes. So we effectively do EoS independently in like 8 places (or more).

sbryngelson avatar Nov 12 '21 21:11 sbryngelson

The probably is probably best addressed (after discussion with @anandrdbz) by creating pointers, assigned in initialize_module that point to specific functions for computing the pressure (associated with the E_idx) from conservative variables as it depends on model_eqns, bubbles, etc. For example, we could just have a pointer called s_compute_pressure that points to, e.g., s_compute_pressure_5eq_bubbles(q_cons_vf,p) for the 5 equation model with sub-grid bubbles (with cons. variables input and pressure output). Then we could have a separate subroutine for each specific case.

sbryngelson avatar Nov 15 '21 15:11 sbryngelson

This has similarities to new issue MFlowCode/MFC-develop#18

Thus - it also should probably not be addressed until GPU branch is merged.

sbryngelson avatar Nov 18 '21 23:11 sbryngelson

I have gone ahead and implemented the s_compute_pressure subroutines and pointers, located in m_initial_condition.fpp. However, because m_initial_condition.fpp uses m_variables_conversion.fpp and m_variables_conversion.fpp uses m_initial_condition.fpp, there is an error being thrown:

~/src/pre_process/m_variables_conversion.f90:19:9:

   19 |     use m_initial_condition
      |         1
Error: 'm_variables_conversion' of module 'm_initial_condition', imported at (1), is also the name of the current program unit

Therefore I am unsure if there's any way I can really implement s_compute_pressure in m_initial_condition. Is there any other module that I can try and implement it in?

anshgupta1234 avatar Nov 11 '22 08:11 anshgupta1234

I think m_variable_conversion is a more natural home for this kind of thing.

sbryngelson avatar Nov 11 '22 14:11 sbryngelson

@anshgupta1234 Check out these lines in m_cbc:

https://github.com/MFlowCode/MFC/blob/d80602abfc6c1528708963032ef8cd44b3f26622/src/simulation/m_cbc.fpp#L786-L853

I think these can probably be factored out completely. One thing we didn't initially notice is how pervasive the computation of c, the speed of sound is. This is above, but also throughout the codebase. We should create a helper function that goes in common/m_var_conversion that computes the speed of sound.

We see something similar in m_riemann_solvers, for example: https://github.com/MFlowCode/MFC/blob/d80602abfc6c1528708963032ef8cd44b3f26622/src/simulation/m_riemann_solvers.fpp#L464-L572

We also see the computation of c coming up again. Each time it takes something like 70 lines:

https://github.com/MFlowCode/MFC/blob/d80602abfc6c1528708963032ef8cd44b3f26622/src/simulation/m_riemann_solvers.fpp#L639-L714

sbryngelson avatar Dec 07 '22 13:12 sbryngelson

I realize this is becoming an omnibus "issue". Let's merge your pull request when you have all of the main variables conversion stuff done, then do speed of sound c after.

sbryngelson avatar Dec 07 '22 13:12 sbryngelson