Error if temperature BC or source with F.Temperature

[JonathanGSDUFOUR]

Proposed changes

Added check in the generic_simulation to prevent the use of festim.Temperature with boundary condition set on the T field (Issue #787 ).

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)

Further comments

Some other tests had to be slightly modified, is there a better way than what I did ?

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.55%. Comparing base (a9d6147) to head (aba4540).
Report is 100 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #812   +/-   ##
=======================================
  Coverage   99.55%   99.55%           
=======================================
  Files          61       61           
  Lines        2707     2711    +4     
=======================================
+ Hits         2695     2699    +4     
  Misses         12       12           

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

[RemDelaporteMathurin]

Since this is about the HeatTransferProblem class I'll let @KulaginVladimir review this first (when he has time)

[KulaginVladimir]

Hi @JonathanGSDUFOUR. Thank you for this fix.

I have a few comments on the current implementation below. Moreover, it appears to me that your fix covers only the case when boundary conditions for the temperature field are set simultaniously with F.Temperature, but doesn't catch the cases when a source or an initial condition are set together with F.Temperature which were mentioned at the start of #787. However, I think the latter can be discussed with @RemDelaporteMathurin after his review.

[RemDelaporteMathurin]

I agree with @KulaginVladimir we would need to perform this check also if Source objects are set.

Vladimir, initial temperatures are set as an argument of HeatTransferProblem right? so maybe no need to check this too? idk

[KulaginVladimir]

Vladimir, initial temperatures are set as an argument of HeatTransferProblem right? so maybe no need to check this too? idk

Indeed. There is no need in such a check. I assume that I was thinking of the HTransportProblem and just transferred the thoughts to the HeatTransferProblem. What happens if we conduct a steady-state simulation and set an initial condition for solute or traps?

This is not related to the current PR anyway.

[RemDelaporteMathurin]

What happens if we conduct a steady-state simulation and set an initial condition for solute or traps?

I'm not entirely sure that an error is raised

[JonathanGSDUFOUR]

Thank you @KulaginVladimir and @RemDelaporteMathurin for the feedback !
I started with the test in test_initial_conditions but realised Convective_flux had to be removed from the test_flux_BC_initialise because of the new error message. I will clean the duplication and extend the checks to 'Sources' too.
I'll make sure to learn from those errors !

[RemDelaporteMathurin]

Hey @JonathanGSDUFOUR here are a few very small changes 👍

After that I think it's good to go, unless @KulaginVladimir sees something else

[RemDelaporteMathurin]

I'll let this open for a little while longer just in case someone else has comments

[RemDelaporteMathurin]

I think we can merge this now, @KulaginVladimir

[RemDelaporteMathurin]

Thank you so much for this contribution Jonathan!! 🎉