Repository: PMEAL/OpenPNM

Issue:
Fine-tune iterative props machinery
- [x] Move ``update_iterative_props`` to a higher level, like GenericAlgorithm, or perhaps as a Mixin
- [x] Add functools.cached_property to ``get_iterative_props``
- [ ] Decide if/how to update models on algorithm.  Ex. Drainage could use 'update_iterative_props' method, or could use ``regenerate_models``
- [x] Remove ModelsMixin from GenericAlg inheritance?
- [x] Remove ``set_variable_props`` method, and just rely on adding directly to settings?
- [ ] ~~Expose some of the hidden methods, like ``_update_A_and_b`` so users can see what's actually happening?~~
- [x] Should the get/set/run iterative props methods be public?
- [ ] ~~Remove 'pores' argument for ``set_source`` method, since these are specified by ``domain`` when defining the reaction model?~~

Discussion:

Patch:
diff --git a/openpnm/algorithms/_advection_diffusion.py b/openpnm/algorithms/_advection_diffusion.py
index 2cd6c415b..efdf9cd0d 100644
--- a/openpnm/algorithms/_advection_diffusion.py
+++ b/openpnm/algorithms/_advection_diffusion.py
@@ -47,15 +47,6 @@ class AdvectionDiffusion(ReactiveTransport):
             kwargs['name'] = 'ad_01'
         super().__init__(settings=self.settings, **kwargs)
 
-    @docstr.dedent
-    def _get_settings(self):
-        r"""
-        Dataclass-like attribute for storing settings
-
-        %(AdvectionDiffusionSettings.parameters)s
-        """
-        return super()._get_settings()
-
     def set_outflow_BC(self, pores, mode='merge'):
         r"""
         Adds outflow boundary condition to the selected pores
diff --git a/openpnm/algorithms/_generic_algorithm.py b/openpnm/algorithms/_generic_algorithm.py
index 329983d64..502a9d296 100644
--- a/openpnm/algorithms/_generic_algorithm.py
+++ b/openpnm/algorithms/_generic_algorithm.py
@@ -1,7 +1,8 @@
 import logging
 import numpy as np
-from openpnm.core import Domain
+from openpnm.core import ParserMixin, LabelMixin, Base2
 from openpnm.utils import Docorator, SettingsAttr
+import functools
 
 
 __all__ = ['GenericAlgorithm']
@@ -25,16 +26,13 @@ class GenericAlgorithmSettings:
 
 @docstr.get_sections(base='GenericAlgorithm', sections=['Parameters'])
 @docstr.dedent
-class GenericAlgorithm(Domain):
+class GenericAlgorithm(ParserMixin, LabelMixin, Base2):
     r"""
     Generic class to define the foundation of Algorithms
 
     Parameters
     ----------
-    network : OpenPNM Network
-        The network object to which this algorithm will apply
-    name : str, optional
-        Name of the algorithm. If not provided one is generated.
+    %(Base.parameters)s
 
     """
 
@@ -46,3 +44,52 @@ class GenericAlgorithm(Domain):
         super().__init__(network=network, settings=self.settings, **kwargs)
         self['pore.all'] = np.ones(network.Np, dtype=bool)
         self['throat.all'] = np.ones(network.Nt, dtype=bool)
+
+    # @functools.cached_property
+    @property
+    def iterative_props(self):
+        r"""
+        Finds and returns properties that need to be iterated while
+        running the algorithm.
+        """
+        import networkx as nx
+        phase = self.project[self.settings.phase]
+        # Generate global dependency graph
+        dg = nx.compose_all([x.models.dependency_graph(deep=True)
+                             for x in [phase]])
+        variable_props = self.settings["variable_props"].copy()
+        variable_props.add(self.settings["quantity"])
+        base = list(variable_props)
+        # Find all props downstream that depend on base props
+        dg = nx.DiGraph(nx.edge_dfs(dg, source=base))
+        if len(dg.nodes) == 0:
+            return []
+        iterative_props = list(nx.dag.lexicographical_topological_sort(dg))
+        # "variable_props" should be in the returned list but not "quantity"
+        if self.settings.quantity in iterative_props:
+            iterative_props.remove(self.settings["quantity"])
+        return iterative_props
+
+    def _update_iterative_props(self, iterative_props=None):
+        """
+        Regenerates phase, geometries, and physics objects using the
+        current value of ``quantity``.
+
+        Notes
+        -----
+        The algorithm directly writes the value of 'quantity' into the
+        phase, which is against one of the OpenPNM rules of objects not
+        being able to write into each other.
+
+        """
+        if iterative_props is None:
+            iterative_props = self.iterative_props
+        if not iterative_props:
+            return
+        # Fetch objects associated with the algorithm
+        phase = self.project[self.settings.phase]
+        # Update 'quantity' on phase with the most recent value
+        quantity = self.settings['quantity']
+        phase[quantity] = self.x
+        # Regenerate all associated objects
+        phase.regenerate_models(propnames=iterative_props)
diff --git a/openpnm/algorithms/_generic_transport.py b/openpnm/algorithms/_generic_transport.py
index 2b6ce22ee..6324ee0d2 100644
--- a/openpnm/algorithms/_generic_transport.py
+++ b/openpnm/algorithms/_generic_transport.py
@@ -28,13 +28,19 @@ class GenericTransportSettings:
     ----------
     %(GenericAlgorithmSettings.parameters)s
     quantity : str
-        The name of the physical quantity to be calculated
+        The name of the physical quantity to be solved for (i.e.
+        'pore.concentration')
     conductance : str
-        The name of the pore-scale transport conductance values. These are
-        typically calculated by a model attached to a *Physics* object
-        associated with the given *Phase*.
+        The name of the pore-scale transport conductance values (i.e
+        'throat.diffusive_conductance')
     cache : bool
-        If ``True``, A matrix is cached and rather than getting rebuilt.
+        If ``True``, the ``A`` matrix is cached and rather than getting
+        rebuilt.
+    variable_props : list of strings
+        This list (actually a set) indicates which properties are variable
+        and should be updated by the algorithm on each iteration. Note that
+        any properties which already depend on ``'quantity'`` will
+        automatically be updated.
 
     """
     phase = ''
@@ -95,12 +101,8 @@ class GenericTransport(GenericAlgorithm, BCsMixin):
 
         """
         gvals = self.settings['conductance']
-        # FIXME: this needs to be properly addressed (see issue #1548)
-        try:
-            if gvals in self._get_iterative_props():
-                self.settings.cache = False
-        except AttributeError:
-            pass
+        if gvals in self.iterative_props:
+            self.settings.cache = False
         if not self.settings['cache']:
             self._pure_A = None
         if self._pure_A is None:
@@ -404,40 +406,3 @@ class GenericTransport(GenericAlgorithm, BCsMixin):
                 R = np.sum(R)
 
         return np.array(R, ndmin=1)
-
-    def set_variable_props(self, variable_props, mode='add'):
-        r"""
-        This method is useful for setting variable_props to the settings
-        dictionary of the target object. Variable_props and their dependent
-        properties get updated iteratively.
-
-        Parameters
-        ----------
-        variable_props : str, or List(str)
-            A single string or list of strings to be added as variable_props
-        mode : str, optional
-            Controls how the variable_props are applied. The default value is
-            'add'. Options are:
-
-            ===========  =====================================================
-            mode         meaning
-            ===========  =====================================================
-            'add'        Adds supplied variable_props to already existing list
-                         (if any), and prevents duplicates
-            'overwrite'  Deletes all exisitng variable_props and then adds
-                         the specified new ones
-            ===========  =====================================================
-
-        """
-        # If single string, make it a list
-        if isinstance(variable_props, str):
-            variable_props = [variable_props]
-        # Handle mode
-        mode = self._parse_mode(mode, allowed=['add', 'overwrite'],
-                                single=True)
-        if mode == 'overwrite':
-            self.settings['variable_props'].clear()
-        # parse each propname and append to variable_props in settings
-        for variable_prop in variable_props:
-            variable_prop = self._parse_prop(variable_prop, 'pore')
-            self.settings['variable_props'].add(variable_prop)
diff --git a/openpnm/algorithms/_reactive_transport.py b/openpnm/algorithms/_reactive_transport.py
index 6d65658d4..6d57b2be7 100644
--- a/openpnm/algorithms/_reactive_transport.py
+++ b/openpnm/algorithms/_reactive_transport.py
@@ -116,29 +116,6 @@ class ReactiveTransport(GenericTransport):
         if propname not in self.settings['sources']:
             self.settings['sources'].append(propname)
 
-    def _update_iterative_props(self):
-        """
-        Regenerates phase, geometries, and physics objects using the
-        current value of ``quantity``.
-
-        Notes
-        -----
-        The algorithm directly writes the value of 'quantity' into the
-        phase, which is against one of the OpenPNM rules of objects not
-        being able to write into each other.
-
-        """
-        iterative_props = self._get_iterative_props()
-        if not iterative_props:
-            return
-        # Fetch objects associated with the algorithm
-        phase = self.project[self.settings.phase]
-        # Update 'quantity' on phase with the most recent value
-        quantity = self.settings['quantity']
-        phase[quantity] = self.x
-        # Regenerate all associated objects
-        phase.regenerate_models(propnames=iterative_props)
-
     def _apply_sources(self):
         """
         Updates ``A`` and ``b``, applying source terms to specified pores.
@@ -243,29 +220,6 @@ class ReactiveTransport(GenericTransport):
         super()._update_A_and_b()
         self._apply_sources()
 
-    def _get_iterative_props(self):
-        r"""
-        Finds and returns properties that need to be iterated while
-        running the algorithm.
-        """
-        import networkx as nx
-        phase = self.project[self.settings.phase]
-        # Generate global dependency graph
-        dg = nx.compose_all([x.models.dependency_graph(deep=True)
-                             for x in [phase]])
-        variable_props = self.settings["variable_props"].copy()
-        variable_props.add(self.settings["quantity"])
-        base = list(variable_props)
-        # Find all props downstream that depend on base props
-        dg = nx.DiGraph(nx.edge_dfs(dg, source=base))
-        if len(dg.nodes) == 0:
-            return []
-        iterative_props = list(nx.dag.lexicographical_topological_sort(dg))
-        # "variable_props" should be in the returned list but not "quantity"
-        if self.settings.quantity in iterative_props:
-            iterative_props.remove(self.settings["quantity"])
-        return iterative_props
-
     def _get_residual(self, x=None):
         r"""
         Calculates solution residual based on the given ``x`` based on the
diff --git a/openpnm/core/_base2.py b/openpnm/core/_base2.py
index 13d85a081..f238e099e 100644
--- a/openpnm/core/_base2.py
+++ b/openpnm/core/_base2.py
@@ -1,13 +1,25 @@
 import numpy as np
-from openpnm.core import LabelMixin, ModelsDict, ModelWrapper, ParserMixin
-from openpnm.utils import Workspace, SettingsAttr, PrintableList, PrintableDict
-from openpnm.utils import parse_mode
+from openpnm.core import (
+    LabelMixin,
+    ModelsDict,
+    ModelWrapper,
+    ParserMixin,
+)
+from openpnm.utils import (
+    Workspace,
+    SettingsAttr,
+    PrintableList,
+    PrintableDict,
+    Docorator,
+    parse_mode,
+)
 from copy import deepcopy
 import inspect
 import uuid
 import numpy.lib.recfunctions as rf
 
 
+docstr = Docorator()
 ws = Workspace()
 
 
@@ -17,6 +29,8 @@ __all__ = [
 ]
 
 
+@docstr.get_sections(base='BaseSettings', sections=docstr.all_sections)
+@docstr.dedent
 class BaseSettings:
     r"""
     The default settings to use on instance of Base
@@ -30,7 +44,17 @@ class BaseSettings:
     uuid = ''
 
 
+@docstr.get_sections(base='Base', sections=['Parameters'])
+@docstr.dedent
 class Base2(dict):
+    r"""
+    Base class
+
+    Parameters
+    ----------
+    network : dict
+        An OpenPNM Network object, which is a subclass of a dict
+    """
 
     def __init__(self, network=None, settings=None, name='obj'):
         super().__init__()
diff --git a/openpnm/core/_models.py b/openpnm/core/_models.py
index 90c6712bf..5e3eef308 100644
--- a/openpnm/core/_models.py
+++ b/openpnm/core/_models.py
@@ -217,7 +217,7 @@ class ModelWrapper(dict):
     functionality, such as pretty-printing and the ability to run itself.
     """
     @property
-    def propname(self):
+    def name(self):
         for proj in ws.values():
             for obj in proj:
                 if hasattr(obj, 'models'):
@@ -225,6 +225,16 @@ class ModelWrapper(dict):
                         if mod is self:
                             return key
 
+    @property
+    def propname(self):
+        return self.name.split('@')[0]
+
+    @property
+    def domain(self):
+        element, prop = self.name.split('.', 1)
+        prop, domain = prop.split('@')
+        return element + '.' + domain
+
     def __str__(self):
         horizontal_rule = '―' * 78
         lines = [horizontal_rule]


Test patch:
diff --git a/tests/unit/algorithms/ReactiveTransportTest.py b/tests/unit/algorithms/ReactiveTransportTest.py
index e3bdcd92d..af88425a6 100644
--- a/tests/unit/algorithms/ReactiveTransportTest.py
+++ b/tests/unit/algorithms/ReactiveTransportTest.py
@@ -116,7 +116,7 @@ class ReactiveTransportTest:
         self.alg['pore.bc_value'] = np.nan
         _ = [self.alg.__setitem__(k, False) for k in self.alg.settings.sources]
         self.alg.set_source(pores=self.net.pores('left'), propname='pore.reaction')
-        assert "pore.reaction" in self.alg._get_iterative_props()
+        assert "pore.reaction" in self.alg.iterative_props
 
     def test_quantity_relaxation_consistency_w_base_solution(self):
         self.alg['pore.bc_rate'] = np.nan
diff --git a/tests/unit/algorithms/TransientMultiPhysicsTest.py b/tests/unit/algorithms/TransientMultiPhysicsTest.py
index 807c54601..9a9654abb 100644
--- a/tests/unit/algorithms/TransientMultiPhysicsTest.py
+++ b/tests/unit/algorithms/TransientMultiPhysicsTest.py
@@ -70,8 +70,8 @@ class TransientMultiPhysicsTest:
         self.c0[self.net.pores('left')] = 100
 
         # add variable props to algs
-        self.tfd.set_variable_props('pore.temperature')
-        self.tfc.set_variable_props('pore.concentration')
+        self.tfd.settings.variable_props.add('pore.temperature')
+        self.tfc.settings.variable_props.add('pore.concentration')
 
         # multiphysics algorithm
         algs = [self.tfc, self.tfd]