Total Volume for Cylindrical and Spherical meshes

[rekomodo]

Proposed changes

Implements TotalVolume derived quantities for both Cylindrical and Spherical meshes as per #723 .

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

If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered, etc...

[codecov]

Codecov Report

Attention: Patch coverage is 96.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 99.49%. Comparing base (abe3942) to head (9dfb219).

Files with missing lines	Patch %	Lines
festim/exports/derived_quantities/total_volume.py	95.91%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #882      +/-   ##
==========================================
- Coverage   99.56%   99.49%   -0.07%     
==========================================
  Files          61       61              
  Lines        2750     2799      +49     
==========================================
+ Hits         2738     2785      +47     
- Misses         12       14       +2     

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

[RemDelaporteMathurin]

Hi @rekomodo thanks for this contribution it will be much needed!

I made a few comments. The expression for spherical is wrong and needs fixing

[RemDelaporteMathurin]

Suggested change

	int(f r dx)
	int(f r dr)

[RemDelaporteMathurin]

For these new classes you will need to override the export_unit property.

The units of TotalVolumeCylindrical and TotalVolumeSpherical are always H (since it's a 3D equivalent).

For TotalVolume this property is defined here:

FESTIM/festim/exports/derived_quantities/total_volume.py

Lines 38 to 50 in 9dfb219

	@property
	def export_unit(self):
	# obtain domain dimension
	try:
	dim = self.function.function_space().mesh().topology().dim()
	except AttributeError:
	dim = self.dx._domain._topological_dimension
	# TODO we could simply do that all the time
	# return unit depending on field and dimension of domain
	if self.field == "T":
	return f"K m{dim}".replace("1", "")
	else:
	return f"H m{dim-3}".replace(" m0", "")

[RemDelaporteMathurin]

This line isn't tested. Could we add a simple test that checks that the only allowed mesh is spherical?

[RemDelaporteMathurin]

Suggested change

	int(f r dx)
	int(f r**2 dx)

Btw, this should include the angular coverages too! Same comment for the Cylindrical version

[RemDelaporteMathurin]

This is not correct. There should be a cos somewhere (see this).

Consider the expression of the volume element in spherical coordinates (link):

[RemDelaporteMathurin]

This is not the expected value, see my comment above