festim-dev / FESTIM

Coupled hydrogen/tritium transport and heat transfer modelling using FEniCS
https://festim.readthedocs.io/en/stable/
Apache License 2.0
92 stars 24 forks source link

Correct units for cylindrical and spherical fluxes #835

Closed RemDelaporteMathurin closed 3 months ago

RemDelaporteMathurin commented 3 months ago

Proposed changes

Fix for #828

We should add this units property to all derived quantities!

Types of changes

What types of changes does your code introduce to FESTIM?

Checklist

codecov[bot] commented 3 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 99.56%. Comparing base (10bfae9) to head (4e45b4a). Report is 68 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #835 +/- ## ======================================= Coverage 99.56% 99.56% ======================================= Files 61 61 Lines 2747 2754 +7 ======================================= + Hits 2735 2742 +7 Misses 12 12 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

RemDelaporteMathurin commented 3 months ago

Could take the opportunity to add the attributes to the child classes, but not necessary

What child classes?

jhdark commented 3 months ago

Could take the opportunity to add the attributes to the child classes, but not necessary

What child classes?

SurfaceFluxCylindrical and SurfaceFluxSpherical

RemDelaporteMathurin commented 3 months ago

Could take the opportunity to add the attributes to the child classes, but not necessary

What child classes?

SurfaceFluxCylindrical and SurfaceFluxSpherical

Well that's already what this PR is doing 😕