festim-dev / festim-dev/FESTIM
Move the coordinate system from the mesh to the volume subdomain
- Dominant language
- Python
- Stars
- 135
- Forks
- 45
- Avg merge
- 3d 23h
- Merged PRs (30d)
- 14
Description
`coordinate_system` is currently a property of the mesh, so a whole model is either cartesian, cylindrical or spherical. It should live on `VolumeSubdomain` instead, so that e.g. a spherical pebble and a cylindrical pipe wall can sit in one model, on disconnected `Mesh1D` blocks coupled by an enclosure.
```python
pebble = F.VolumeSubdomain1D(id=1, borders=[0, 1e-3], material=mat_1,
coordinate_system="spherical")
pipe = F.VolumeSubdomain1D(id=2, borders=[5e-3, 6e-3], material=mat_2,
coordinate_system="cylindrical")
```
Single home, not both: `Mesh(coordinate_system=...)` becomes a thin deprecated kwarg that emits a `DeprecationWarning` and applies to subdomains that set nothing, removed after one release.
## Why it's a small change
The metric factor never enters the measure — the diffusion term is written `r * dot(D*grad(u), grad(v/r)) * dx`, so the factors cancel in every zeroth- and first-order term (transient, sources, fluxes, advection carry no `r` today). **Only the diffusion terms need the flag**, and they already sit inside per-subdomain loops.
## What to change
- **A metric-factor helper**, e.g. `CoordinateSystem.metric_factor(mesh)` returning `1` / `r` / `r**2` ([mesh.py:12-40](src/festim/mesh/mesh.py#L12-L40)). Each call site becomes `w * dot(D*grad(u), grad(v/w)) * dx`, correct for cartesian too. Kills three verbatim copies of the same `match` block.
- **Call sites**: `HydrogenTransportProblemDiscontinuous` ([:1632](src/festim/hydrogen_transport_problem.py#L1632)) and `HydrogenTransportProblemDG`
([:2758](src/festim/hydrogen_transport_problem.py#L2758)). The copies in `HydrogenTransportProblem` ([:911](src/festim/hydrogen_transport_problem.py#L911)) and `...ChangeVar` ([:2441](src/festim/hydrogen_transport_problem.py#L2441)) are on the deprecation path — leave them on the deprecated mesh-level flag.
- **`VolumeSubdomain`**: new kwarg + setter accepting `str | CoordinateSystem` ([volume_subdomain.py:49](src/festim/subdomain/volume_subdomain.py#L49), forwarded from `VolumeSubdomain1D` [:143](src/festim/subdomain/volume_subdomain.py#L143)). `CoordinateSystem` is not re-exported in
[`__init__.py`](src/festim/__init__.py#L83) — it should be.
- **Validation**: `check_mesh_dim_coords` ([mesh.py:228](src/festim/mesh/mesh.py#L228)) moves to a per-subdomain check at `initialise()`. Add: subdomains sharing a conforming `Interface` must have the same coordinate system (mismatched metrics break flux continuity) — different symmetries connect through disconnected blocks + an enclosure.
- **Two blanket guards** reject the whole model on the mesh flag and must be re-pointed at the subdomain the quantity/enclosure belongs to: derived quantities [:493](src/festim/hydrogen_transport_problem.py#L493), enclosures [:1714](src/festim/hydrogen_transport_problem.py#L1714).
Contributor guide
No contributing guide indexed for this repository
Research direction
Start with CoordinateSystem and mesh.py, then inspect VolumeSubdomain and VolumeSubdomain1D in volume_subdomain.py. Trace the diffusion call sites in hydrogen_transport_problem.py, along with the derived-quantity and enclosure guards. Done means subdomains own the coordinate system, conforming interfaces validate matching systems, the deprecated mesh kwarg still applies as specified, and CoordinateSystem is re-exported.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- backend
- Issue type
- Feature
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 68/100