Custom derived quantity

[RemDelaporteMathurin]

Description

Summary

This PR adds festim.exports.CustomQuantity which allows users to compute any derived quantity on a volume or surface subdomain.

Other changes:

• VolumeQuantity and SurfaceQuantity now both inherit from DerivedQuantity (all three are abstract classes)
• This allows to deduplicate code like write() and filename()
• added facet_meshtags and volume mesthtags attributes to MaximumVolume (and friends)
• I had to modify the way we do the "D_global" thing in the continuous problem to make sure that kwargs["D"] was updated when the temperature was time dependent

Usage

import numpy as np
import festim as F

material = F.Material(D_0=1, E_D=0)
volume = F.VolumeSubdomain(id=1, material=material)
surface = F.SurfaceSubdomain(id=1, locator=lambda x: np.isclose(x[1], 1))


# cA + cB
def total_concentration(**kwargs):
    return kwargs["A"] + kwargs["B"]

quantity = F.CustomQuantity(
    expr=total_concentration,
    subdomain=volume,
    title="Total quantity",
)

# -D grad(c_A) . n
surface_quantity = F.CustomQuantity(
    expr=lambda **kwargs: -kwargs["D_A"] * ufl.dot(
        ufl.grad(kwargs["A"]), kwargs["n"]
    ),
    subdomain=surface,
    title="Surface flux",
)

Related Issues

Fixes #1130

Motivation and Context

Type of Change

• 🐛 Bug fix (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 (no functional changes, no API changes)
• 📝 Documentation update
• ✅ Test update (adding missing tests or correcting existing tests)
• 🔧 Build/CI configuration change

Testing

• All existing tests pass locally (pytest)
• I have added new tests that prove my fix is effective or that my feature works

Code Quality Checklist

• My code follows the code style of this project (Ruff formatted: ruff format .)
• My code passes linting checks (ruff check .)
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas

Documentation

• I have updated the documentation accordingly (if applicable)
• I have added docstrings to new functions/classes following the project conventions

Breaking Changes

Screenshots/Examples

Additional Notes

[codecov]

Codecov Report

❌ Patch coverage is 93.16770% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 94.72%. Comparing base (b8ec294) to head (bed9596).

Files with missing lines	Patch %	Lines
src/festim/exports/surface_quantity.py	72.72%	3 Missing ⚠️
src/festim/exports/volume_quantity.py	75.00%	3 Missing ⚠️
src/festim/exports/derived_quantity.py	95.12%	2 Missing ⚠️
src/festim/hydrogen_transport_problem.py	96.96%	2 Missing ⚠️
src/festim/exports/custom_quantity.py	95.65%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1138      +/-   ##
==========================================
- Coverage   94.93%   94.72%   -0.21%     
==========================================
  Files          44       46       +2     
  Lines        3434     3508      +74     
==========================================
+ Hits         3260     3323      +63     
- Misses        174      185      +11     

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[jhdark]

Looks good to me, is it worth adding some testing too to make sure things break when trying to define an volume/surface quantity without giving a title/compute functions? Although this isnt FESTIM functionality just the python abstract class.

[jhdark]

These math blocks render on the docs page, but not with the VSC hover/tooltip function. Maybe this is fine? Otherwise, we could just have the equations in Unicode?

[RemDelaporteMathurin]

I think we have this in other places too. I didn't know they don't render on the VSC (pylance?) tooltip. That can be a separate issue that we fix (or not) globally

[jhdark]

Maybe we could make this a required arg instead? happy either way

[RemDelaporteMathurin]

I personally never use titles for derived quantities (even the ones given by default), don't know about the others. That's also because I pretty much never write derived quantities to csv.

For users using derived quantities like me, i would find it a bit constraining to have to add a title even if you never use it. The only downside of having this default title is that if a user exports two custom quantities to a csv then they would both be written as "Custom Quantity" in the header.

Two solutions:

• raise a warning if two quantities have the same name (i'd do it in another PR)
• dynamically add an index to the title if there are several (ie. "Custom Quantity_1", "Custom Quantity_2")

What do you think?

[jhdark]

Tbf, you can give different filenames per quantity too, so maybe not an issue, think its an issue you'd run into once and then change. Dynamic naming would be better, but only if this rare scenario happens, and it would add more logic. Let's just leave it and see what issues we run into, if any