Upstream CheMFC infrastructure

[henryleberre]

Description

Many changes are needed to implement chemistry and some are quite significant. This PR upstreams the core set of modifications that relate to code infrastructure. The PR also includes some non-chemistry, nice-to-have, updates and features.

• New feature (non-breaking change which adds functionality)

Scope

• This PR comprises a set of related changes with a common goal

How Has This Been Tested?

• Local OSX ./mfc.sh test.
• GitHub CI.

What is important here is not the correctness of the chemistry, but that these additions don't break existing code.

[sbryngelson]

Can you add a brief summary of the added features/code cleaning? I realize this is a bit tedious but scrolling through the changed code is harder...

[sbryngelson]

@henryleberre FYI the Phoenix runners will be down for a few days, the computer is under maintenance.

[henryleberre]

List of changes:

• (Main) MFC <-> Pyrometheus compile-time integration (case setup, builds, code-gen, etc.)
• (Main) Parts of m_chemistry, some separation of logic for chemistry/non-chemistry cases at compile-time, variable buffers for chemistry, and some early logic like EoS, reactions, and post_processing.
• (Minor) Creation of ./mfc.sh spelling.
• (Minor) Multi-threaded misc/viz.py.
• (Minor) Creation of m_finite_differences.
• (Minor) Updated indices.dat to include the cons/prim variables, including mass fractions.
• (Minor) Fixed s_rectangle patch for immersed boundaries. Note: I forget how I encountered this. Might have been with a test.
• (Minor) Explicitly zero-out rhs_vf at the start of s_compute_rhs.
• (Minor) Fixed the computation time_avg in m_rhs so that it works in 1D and 2D (not just 3D).
• (Minor) Multi-threaded pip install if you specify -j (could speed up Cantera builds).
• (Minor) Created ./mfc.sh spelling (called by CI).

\dots and I think that's most of them. You can wait for the GT runners to come back up to merge this.

[sbryngelson]

List of changes:

• (Main) MFC <-> Pyrometheus compile-time integration (case setup, builds, code-gen, etc.)
• (Main) Parts of m_chemistry, some separation of logic for chemistry/non-chemistry cases at compile-time, variable buffers for chemistry, and some early logic like EoS, reactions, and post_processing.
• (Minor) Creation of ./mfc.sh spelling.
• (Minor) Multi-threaded misc/viz.py.
• (Minor) Creation of m_finite_differences.
• (Minor) Updated indices.dat to include the cons/prim variables, including mass fractions.
• (Minor) Fixed s_rectangle patch for immersed boundaries. Note: I forget how I encountered this. Might have been with a test.
• (Minor) Explicitly zero-out rhs_vf at the start of s_compute_rhs.
• (Minor) Fixed the computation time_avg in m_rhs so that it works in 1D and 2D (not just 3D).
• (Minor) Multi-threaded pip install if you specify -j (could speed up Cantera builds).
• (Minor) Created ./mfc.sh spelling (called by CI).

\dots and I think that's most of them. You can wait for the GT runners to come back up to merge this.

Fancy. Will do a little code review and merge once runners are up.

[codecov]

Codecov Report

Attention: Patch coverage is 33.87978% with 121 lines in your changes missing coverage. Please review.

Project coverage is 54.36%. Comparing base (3cf7fb6) to head (3b53b1b).
Report is 2 commits behind head on master.

Files with missing lines	Patch %	Lines
src/common/m_finite_differences.fpp	10.41%	41 Missing and 2 partials ⚠️
src/simulation/m_chemistry.fpp	0.00%	32 Missing ⚠️
src/common/m_variables_conversion.fpp	30.43%	14 Missing and 2 partials ⚠️
src/simulation/m_data_output.fpp	22.22%	14 Missing ⚠️
src/pre_process/m_data_output.fpp	76.92%	1 Missing and 5 partials ⚠️
src/post_process/m_start_up.f90	0.00%	5 Missing ⚠️
src/pre_process/m_patches.fpp	0.00%	5 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #544      +/-   ##
==========================================
- Coverage   54.69%   54.36%   -0.34%     
==========================================
  Files          59       61       +2     
  Lines       13662    13747      +85     
  Branches     1698     1720      +22     
==========================================
+ Hits         7473     7474       +1     
- Misses       5740     5817      +77     
- Partials      449      456       +7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[sbryngelson]

new_patch.patch
did you apply this patch? some of it is needed for CCE18 I think...

[henryleberre]

@sbryngelson I did but it needed some modifications to work with other compilers. I'm fixing the new errors now.

[henryleberre]

@sbryngelson You haven't approved the PR yet. Let me know if you want anything changed.

[sbryngelson]

I think some documentation may need to be updated. For example, the variables here: https://mflowcode.github.io/documentation/md_visualization.html

[henryleberre]

I think some documentation may need to be updated. For example, the variables here: https://mflowcode.github.io/documentation/md_visualization.html

Since this PR adds the infrastructure for chemistry and does not claim to have working chemistry, I actively think we should refrain from documenting the existence of chemistry so as not to confuse users.

[sbryngelson]

Phoenix is down, so I might have to wait to merge this. It looks like Frontier is still not working and it is interesting the Phoenix GPU test suite that finished before the servers went down showed a marked increase in performance.

[sbryngelson]

@henryleberre I suppose this is up next, once conflicts are resolved? I merged #620

[sbryngelson]

This is the first almost-mergable PR since #619. It's failing the CI cleanliness check, which is helpful, but in the diff it does find things like this (I was just perusing the "everything diff":

<   766 |         real(kind(0d0)) :: start, finish
---
>   261 |         real(kind(0d0)) :: start, finish
2781c2410
<   766 |         real(kind(0d0)) :: start, finish
---
>   261 |         real(kind(0d0)) :: start, finish
2820,2823d2448

We should probably change the workflow so that strip line numbers or some such before doing the diff. This is a real warning in the sense that those variables are probably not used, but it doesn't make sense to fail the check just because the user moved the subroutine down a few lines or something. @okBrian .

[henryleberre]

@sbryngelson Do I need to pass the Cleanness CI to merge?

[sbryngelson]

I'd like to see what's involved in passing this test (barring the silly diff warnings, like the line change I commented on above). if no one can actually pass the test, then it needs to be adjusted somehow or removed.

[henryleberre]

@sbryngelson I'm sure I can remove the new warnings but a few are them are spurious since the testing is done with the chemistry parameter set to False. So everything that's behind chemistry = T causes warnings for being unused. Also, even if I got the new warnings to go away, the CI would still have to be updated to ignore the line number where the warnings were produced on.

[sbryngelson]

ok then we can ignore it