Reactions in formulation

[RemDelaporteMathurin]

Proposed changes

This PR is a step towards #598

It integrates the Reaction class in HydrogenTransportProblem by using the reaction term in the formulation

We tried adding tests for the actual formulation in HydrogenTransportProblem but ran into roadblocks since we are creating Constant objects. It's gonna be very hard to test this until we have a way of getting these objects.

Soon, we will be able to add MMS tests. We'll maybe do this in another PR

TODO:

• Testing
• Move the examples

Types of changes

What types of changes does your code introduce to FESTIM?

• Bugfix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Code refactoring
• Documentation Update (if none of the other choices apply)
• New tests

Checklist

• Black formatted
• Unit tests pass locally with my changes
• I have added tests that prove my fix is effective or that my feature works
• I have added necessary documentation (if appropriate)

[codecov]

Codecov Report

Attention: 5 lines in your changes are missing coverage. Please review.

Comparison is base (f18255e) 98.84% compared to head (7a5d7df) 98.34%.

Files	Patch %	Lines
festim/hydrogen_transport_problem.py	75.00%	5 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           fenicsx     #633      +/-   ##
===========================================
- Coverage    98.84%   98.34%   -0.51%     
===========================================
  Files           20       20              
  Lines          869      905      +36     
===========================================
+ Hits           859      890      +31     
- Misses          10       15       +5     

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

[RemDelaporteMathurin]

@gabriele-ferrero, @jhdark and I contributed quite a lot to this PR, would you mind reviewing it?

[gabriele-ferrero]

Thank you Rémi for the request, it seems a good job to me.
Some minor comments we can discuss on:
the volume id as an attribute of reaction is a good way to address the problem, but can it be possible to initialize it to each volume in the scenario where it is not specified?
The reaction class can be a valid method to simulate multitrapping, as shown in the multi isotope trapping tutorial. I would recommend updating the documentation and a tutorial when this version of Festim can handle it, as it can be confusing for a first-time user. For example, when evaluating the total retention, a weight of 2 must be accounted for to the concentration Trapped_H2. For a similar reason (yet, it is an exceptional case) I would suggest calling the multi isotope trap trapped_HD11 instead of trapped_HD
Last, an energy term can be added to the reaction term. As Festim is focused on hydrogen isotopes, I see the reason not to do this, but if you want to add up on capabilities it can be something to consider. Of course it can be defined as an optional argument of the reaction class, that if not specified is 0.

[RemDelaporteMathurin]

the volume id as an attribute of reaction is a good way to address the problem, but can it be possible to initialize it to each volume in the scenario where it is not specified?

That's a very good call. I think we should open an issue to track the progress on this and address this in another PR as the functionality is currently working

The reaction class can be a valid method to simulate multitrapping, as shown in the multi isotope trapping tutorial. I would recommend updating the documentation and a tutorial when this version of Festim can handle it, as it can be confusing for a first-time user. For example, when evaluating the total retention, a weight of 2 must be accounted for to the concentration Trapped_H2. For a similar reason (yet, it is an exceptional case) I would suggest calling the multi isotope trap trapped_HD11 instead of trapped_HD

That's also a very good point. I think we'll start the documentation work soon-ish but this will be added to the tutorials/workshop for sure

Last, an energy term can be added to the reaction term.

By energy term do you mean like heating rate for exothermic/endothermic? If so that would completely change the way we couple things in FESTIM. The current coupling between heat transfer and hydrogen transport is weak, but this would make it a strong coupling where the H transport has an impact on the heat transfer.

This is an interesting remark but maybe outside of the scope of FESTIM. What do you think @jhdark ?