Skip to content

Add solver barrier for VEP and fix is_viscoplastic #95

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lmoresi merged 2 commits into from
Mar 30, 2026
Merged

Add solver barrier for VEP and fix is_viscoplastic #95

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ba4f349
Add solver barrier for VEP and fix is_viscoplastic comparison
lmoresi Mar 26, 2026
b5d57a2
Update test_1050 to use VE_Stokes for viscoelastic models
lmoresi Mar 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 16 additions & 1 deletion src/underworld3/constitutive_models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -579,6 +579,16 @@ def _reset(self):

return

@property
def requires_stress_history(self):
"""Whether this model needs DFDt stress history tracking.

Models that return True require a solver with stress history
management (e.g. VE_Stokes). Assigning such a model to a plain
Stokes solver will raise an error.
Comment on lines +584 to +588

Copilot AI Mar 26, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docstring says assigning a stress-history model to a plain Stokes solver “will raise an error”, but the enforcement depends on solver-side checks and can be bypassed (e.g. if Unknowns.DFDt is non-None). Consider rewording this to reflect the actual contract (e.g. “requires a solver that manages stress history such as VE_Stokes; plain Stokes is unsupported”) so it stays accurate even if the solver-side guard changes.

Suggested change
"""Whether this model needs DFDt stress history tracking.
Models that return True require a solver with stress history
management (e.g. VE_Stokes). Assigning such a model to a plain
Stokes solver will raise an error.
"""Whether this model needs DFDt stress-history tracking.
Models that return True require a solver that manages stress
history (e.g. VE_Stokes). Using such a model with a plain Stokes
solver is unsupported and may fail depending on the solver
configuration.

Copilot uses AI. Check for mistakes.
"""
return False

def _build_c_tensor(self):
"""Return the identity tensor of appropriate rank (e.g. for projections)"""

Expand Down Expand Up @@ -1588,6 +1598,11 @@ def _object_viewer(self):
## Todo: add all the other properties in here
)

@property
def requires_stress_history(self):
"""VEP models always require stress history tracking."""
return True

@property
def is_elastic(self):
"""True if elastic behavior is active (finite dt_elastic and shear_modulus)."""
Expand All @@ -1604,7 +1619,7 @@ def is_elastic(self):
@property
def is_viscoplastic(self):
"""True if plastic yielding is active (finite yield_stress)."""
if self.Parameters.yield_stress == sympy.oo:
if self.Parameters.yield_stress.sym is sympy.oo:
return False

return True
Expand Down
10 changes: 10 additions & 0 deletions src/underworld3/cython/petsc_generic_snes_solvers.pyx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -950,6 +950,16 @@ class SolverBaseClass(uw_object):
"constitutive_model must be a valid class or instance of a valid class"
)

# Check that the solver can support this constitutive model's requirements.
# Models with stress history (VEP) need a solver that manages DFDt — e.g. VE_Stokes.
# Using them on a plain Stokes solver silently drops the history terms.
if self._constitutive_model.requires_stress_history and self.Unknowns.DFDt is None:
raise TypeError(
f"{type(self._constitutive_model).__name__} requires stress history tracking "
f"(DFDt). Use uw.systems.VE_Stokes instead of uw.systems.Stokes, or provide "
f"a DFDt object when constructing the solver."
Comment on lines +959 to +960

Copilot AI Mar 26, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error message suggests that providing a DFDt object to uw.systems.Stokes is a supported alternative, but plain Stokes doesn’t perform the stress-history DFDt.update_pre_solve()/update_post_solve() sequencing that VE_Stokes does. This can mislead users into thinking history will be handled correctly when it won’t. Consider removing that suggestion, or explicitly stating that manual DFDt management is required if not using VE_Stokes (and/or tightening the check to require a solver that actually manages stress history).

Suggested change
f"(DFDt). Use uw.systems.VE_Stokes instead of uw.systems.Stokes, or provide "
f"a DFDt object when constructing the solver."
f"(DFDt). Use uw.systems.VE_Stokes instead of uw.systems.Stokes. If you "
f"provide a DFDt object when constructing a non-VE solver, you must manually "
f"call DFDt.update_pre_solve() and DFDt.update_post_solve() each timestep."

Copilot uses AI. Check for mistakes.
)
Comment on lines +956 to +961

Copilot AI Mar 26, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This new runtime guard changes behavior for existing code/tests that currently set ViscoElasticPlasticFlowModel on uw.systems.Stokes (e.g. tests/test_1050_VEstokesCart.py uses plain Stokes). Please add/adjust pytest coverage to (a) assert the new TypeError on plain Stokes, and (b) update any existing tests/examples to use VE_Stokes (or to expect the error), so CI doesn’t regress silently.

Copilot uses AI. Check for mistakes.

# May not work due to flux being incomplete
if self.Unknowns.DFDt is not None:
self.Unknowns.DFDt.psi_fn = self._constitutive_model.flux.T
Expand Down
8 changes: 4 additions & 4 deletions tests/test_1050_VEstokesCart.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ def test_stokes_boxmesh(mesh):
)
p = uw.discretisation.MeshVariable(r"mathbf{p}", mesh, 1, vtype=uw.VarType.SCALAR, degree=1)

stokes = uw.systems.Stokes(mesh, velocityField=u, pressureField=p)
stokes = uw.systems.VE_Stokes(mesh, velocityField=u, pressureField=p, order=1)
stokes.constitutive_model = uw.constitutive_models.ViscoElasticPlasticFlowModel
stokes.constitutive_model.Parameters.shear_viscosity_0 = 1
stokes.constitutive_model.Parameters.shear_modulus = 1
Expand Down Expand Up @@ -101,7 +101,7 @@ def test_stokes_boxmesh(mesh):
stokes.add_dirichlet_bc((sympy.oo, 0.0, sympy.oo), "Front")
stokes.add_dirichlet_bc((sympy.oo, 0.0, sympy.oo), "Back")

stokes.solve()
stokes.solve(timestep=0.1)

print(f"Mesh dimensions {mesh.dim}", flush=True)
stokes.dm.ds.view()
Expand Down Expand Up @@ -229,7 +229,7 @@ def test_stokes_boxmesh_bc_failure(mesh):
)
p = uw.discretisation.MeshVariable(r"mathbf{p}", mesh, 1, vtype=uw.VarType.SCALAR, degree=1)

stokes = uw.systems.Stokes(mesh, velocityField=u, pressureField=p)
stokes = uw.systems.VE_Stokes(mesh, velocityField=u, pressureField=p, order=1)
stokes.constitutive_model = uw.constitutive_models.ViscoElasticPlasticFlowModel
stokes.constitutive_model.Parameters.shear_viscosity_0 = 1
stokes.constitutive_model.Parameters.shear_modulus = 1
Expand Down Expand Up @@ -275,7 +275,7 @@ def test_stokes_boxmesh_bc_failure(mesh):
stokes.add_dirichlet_bc((sympy.oo, 0.0, sympy.oo), "Front")
stokes.add_dirichlet_bc((sympy.oo, 0.0, sympy.oo), "Back")

stokes.solve()
stokes.solve(timestep=0.1)

print(f"Mesh dimensions {mesh.dim}", flush=True)
stokes.dm.ds.view()
Expand Down
Loading