Parametrized physical properties #1601

jcapriot · 2025-01-28T06:49:57Z

Summary

Implements a parametrized interface to PhysicalProperties

PR Checklist

If this is a work in progress PR, set as a Draft PR
Linted my code according to the style guides.
Added tests to verify changes to the code.
Added necessary documentation to any new functions/classes following the
expect style.
Marked as ready for review (if this is was a draft PR), and converted
to a Pull Request
Tagged @simpeg/simpeg-developers when ready for review.

Reference issue

What does this implement/fix?

Additional information

Continuing on from #1593

codecov · 2025-01-28T08:52:36Z

Codecov Report

Attention: Patch coverage is 93.27902% with 66 lines in your changes missing coverage. Please review.

Project coverage is 86.55%. Comparing base (2e36896) to head (d97558d).
Report is 29 commits behind head on main.

Files with missing lines	Patch %	Lines
simpeg/props.py	87.57%	26 Missing and 13 partials ⚠️
simpeg/base/pde_simulation.py	88.34%	5 Missing and 7 partials ⚠️
...netics/static/spectral_induced_polarization/run.py	0.00%	6 Missing ⚠️
...agnetics/static/induced_polarization/simulation.py	90.47%	1 Missing and 1 partial ⚠️
simpeg/electromagnetics/base_1d.py	94.73%	0 Missing and 1 partial ⚠️
.../electromagnetics/static/resistivity/simulation.py	50.00%	0 Missing and 1 partial ⚠️
simpeg/electromagnetics/utils/em1d_utils.py	0.00%	0 Missing and 1 partial ⚠️
...etics/viscous_remanent_magnetization/simulation.py	92.30%	0 Missing and 1 partial ⚠️
simpeg/maps/_parametric.py	0.00%	0 Missing and 1 partial ⚠️
simpeg/potential_fields/gravity/simulation.py	87.50%	1 Missing ⚠️
... and 1 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1601      +/-   ##
==========================================
+ Coverage   86.52%   86.55%   +0.02%     
==========================================
  Files         409      410       +1     
  Lines       52692    52984     +292     
  Branches     5000     5058      +58     
==========================================
+ Hits        45594    45860     +266     
- Misses       5678     5698      +20     
- Partials     1420     1426       +6

☔ 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.

Just use an "EM" Simulation

(For now) These should be changed to errors for futures

As suggested in their documentation

jcapriot · 2025-02-02T23:29:11Z

I think the new interface is coming together, here is a summary of the changes:

New interface

On the constructor you can do:

# Initialize with a given value
sim = Simulation(sigma=[1,2,3])

# or initialize as a parametrization (mappings for now)
sim = Simulation(sigma=mapping)

Or changing it after creating a simulation:

# setting the sigma property as a value
sim.sigma = [1, 2, 3]

# parametrizing sigma
sim.parametrize(sigma=mapping)

When properties are parametrized they are dependent on the simulation's model attribute, and derivatives w.r.t. that property are then enabled for inversion. These physical properties also require a model and will error if the simulation doesn't have one assigned.

P.S. A non-invertible property cannot be parametrized

Added methods

These public methods are added to the HasModel class to work together with PhysicalProperties

HasModel.parametrize(**kwargs), the aforementioned item.
HasModel.parametrizations a ParametrizationList storing the mapping used to parametrize each property
- Accessible with attributes named after the PhysicalProperty class.
```
map1 = ExpMap()
sim.simulation(sigma=mapping)
# And it can be retrieved using 
maps1 == sim.parametrizations.sigma
```
HasModel.physical_properties A class method to return a dictionary of attr:PhysicalProperty of all the physical properties defined on a class. There wasn't anything specifically with this functionality before on a class, my hope is that it provides an easy way to see which PhysicalProperties are possible on a simulation.
HasModel.invertible_properties A class method to return a dictionary of attr:PhysicalProperty of all the invertible physical properties defined on a class. This is basically making the previously private class attribute HasModel._all_map_names a public method. The hope is to make it more transparent which physical properties are invertible on a class.
HasModel.is_parametrized(self, attr) Returns a bool indicating if a PhysicalProperty (or it's reciprocal) exists in HasModel.parametrizations. (A note: This is a helper to enable us to check if a derivative is necessary, previously we would do this by checking sim.sigmaMap is None or something similar, this is just a cleaner interface and it also allows us to override it in a subclass.)

Derivatives

Getting the derivative of a property is now handled through a function, as opposed to a property, allowing us to (eventually) pass through multiplication vectors to the parametrizations.

HasModel._prop_deriv(self, attr)

Unless anyone has strong feelings otherwise, I'm in favor of keeping this private to keep the public API cleaner.

I'll add more details in the future

jcapriot · 2025-02-05T21:03:20Z

These are things that would be deprecated or removed with this PR.

Deprecations

On initialization

Simulation(sigmaMap=mapping)` would be deprecated (in favor of the above interface.

Attribute Assignment:

sim.sigmaMap = mapping would go to sim.parametrize(sigma=mapping)

Attribute Access:

mapping = sim.sigmaMap would go to sim.parametrizations.sigma
deriv = sim.sigmaDeriv is no longer an attribute, and is instead a function sim._prop_deriv("sigma", m, v=None).
This is basically a helper for doing sim.parametrizations.sigma.deriv(m, v), that also accounts for reciprocals.

Removals

simpeg.props.Mapping, simpeg.props.Derivative, simpeg.props.Invertible, simpeg.props.Reciprocal. These items were used together with the MetaClass on HasModel to create all of the property attributes that we previously had. As these were used internally and will not work with the new interface, they will throw a SyntaxError if someone attempts to use them.

lheagy

Thanks for bringing this up again @jcapriot and sorry it took me so long to get back to this. I think this idea can work. I have just done a light pass to ask a few questions. Would you mind giving a demo again at an upcoming SimPEG meeting? It would be valuable to share this idea again with the group and get input from others. I think this could simplify things for users, but would appreciate chatting through implications for developers and people working to understand the code

lheagy · 2025-06-18T03:35:00Z

examples/10-pgi/plot_inv_0_PGI_Linear_1D.py

@@ -66,7 +66,7 @@ def g(k):
 mtrue[mesh.cell_centers_x > 0.6] = 0

 # simpeg problem and survey
-prob = simulation.LinearSimulation(mesh, G=G, model_map=maps.IdentityMap())
+prob = simulation.LinearSimulation(G=G, model_map=maps.IdentityMap())


Is this change intended to be in this PR? I am not sure it is in this scope? or maybe I am missing something

lheagy · 2025-06-18T03:35:22Z

examples/10-pgi/plot_inv_1_PGI_Linear_1D_joint_WithRelationships.py

@@ -112,12 +112,12 @@ def g(k):
 relatrive_error = 0.01
 noise_floor = 0.0

-prob1 = simulation.LinearSimulation(mesh, G=G, model_map=wires.m1)
+prob1 = simulation.LinearSimulation(G=G, model_map=wires.m1)


similarly here, are these meant to be a part of this pr? it seems a bit different in scope?

lheagy · 2025-06-18T03:40:30Z

simpeg/props.py

+            val = recip.default
+            # If it wasn't None, try to invert it.
+            if val is not None:
+                val = 1 / val


do we need a check that this is a scalar (or at least isotropic)? This wouldn't work if anisotropic

lheagy · 2025-06-18T03:42:07Z

simpeg/props.py

+            self.fset(obj, value)
+        else:
+            self._fset(obj, value)
+        obj._remove_parametrization(self.name)


if I am following correctly, this would be hit if someone swapped out a mapping with a value or another mapping, is that right? If so, we might want to throw a warning? Changing something from an invertible parameter to not has some potentially involved implications

lheagy · 2025-06-18T03:54:53Z

simpeg/props.py

+            warnings.warn(
+                f"Setting both `{prop1.name}` and `{prop2.name}` to None for `{type(self).__name__}`'s required "
+                f"physical properties is deprecated behavior. This message will be changed to an error in simpeg "
+                f"version X.X",
+                FutureWarning,
+                stacklevel=3,
+                # f"`{type(self).__name__}` requires one of `{prop1.name}` or `{prop2.name}`"
+            )


A question on warning / error handling, will the user first see that they need to change sigmaMap=mapping to sigma=mapping? If not, this warning might be confusing

lheagy · 2025-06-18T03:57:36Z

simpeg/props.py

+        warnings.warn(
+            f"Setting `{cls_name}.{old_map}` directly is deprecated. Instead register a parametrization with `{cls_name}.parametrize('{new_name}')`",
+            FutureWarning,


Can we also add that the user can set the mapping in the class instantiation via prop=mapping. I think that is the behaviour that we will want to encourage (I suspect most people will find the transition from sigmaMap=mapping --> sigma=mapping much easier than going through the class.parameterize ..... approach

jcapriot added 5 commits January 20, 2025 15:34

implement descriptor protocol

d516282

add old class names

e2e302d

add derivative support

4a5986a

update simulation classes

94dcb84

many updates

29a4918

jcapriot marked this pull request as ready for review January 28, 2025 13:51

jcapriot added 14 commits January 28, 2025 14:41

set discretize bugfix release minimum

05e2175

update metas

b8eaa3e

update needs_model

1cd2db0

set default values on the PhysicalProperty

fb6ea9e

fix base PDE test

2c96479

Just use an "EM" Simulation

cleanup props

fb8cb00

ignore deprecation warnings on pytest

194890b

(For now) These should be changed to errors for futures

Replace np.isscalar(x) with np.ndim(x) == 0

07f6074

As suggested in their documentation

fix eigenvalue test

9164cad

fix setters for base pde simulation classes

73aa7bb

more test fixes

6ad24e8

use scipy.integrate.quad

0d340c3

Merge branch 'main' into parametrize

f1bf4a1

make parametrize use **kwargs

d97558d

jcapriot mentioned this pull request Feb 5, 2025

Set physical properties as maps #1593

Closed

6 tasks

jcapriot marked this pull request as draft February 5, 2025 20:51

lheagy reviewed Jun 18, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Parametrized physical properties #1601

Parametrized physical properties #1601

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Parametrized physical properties #1601

Are you sure you want to change the base?

Parametrized physical properties #1601

Uh oh!

Conversation

Uh oh!

Summary

PR Checklist

Reference issue

What does this implement/fix?

Additional information

Uh oh!

Uh oh!

Codecov Report

Uh oh!

Uh oh!

New interface

Added methods

Derivatives

Uh oh!

Uh oh!

Deprecations

Removals

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!