Implementation for POEM_022 - Determining variable shape at runtime based on connections #1671

naylor-b · 2020-09-08T17:26:06Z

Summary

Implementation PR for POEM_022. Adds the ability to determine variable shapes at runtime based on what those variables are connected to. Also adds a new function hook, setup_partials, that is used to allow a component to compute its partials after all shape information is known.

Backwards incompatibilities

None

New Dependencies

None

…n_sizing

…graph

…n_shape

…ch during dyn shape determination

…cally sized variables

…n_shape

Kenneth-T-Moore · 2020-09-09T16:03:33Z

openmdao/core/tests/test_dyn_sizing.py

+        p = om.Problem()
+        indep = p.model.add_subsystem('indep', om.IndepVarComp('x1', val=np.ones((2,3))))
+        indep.add_output('x2', val=np.ones((4,2)))
+        p.model.add_subsystem('Gdyn', DynShapeGroupSeries(3, 2, DynShapeComp))


Have you tested what happens if you accidentally set shape_by_conn to your own name? I imagine you will just get a 'cannot resolve shape' exception, so probably fine.

Yeah, doing a copy_shape using your own name will either result in a failure to resolve, or if that variable happens to be connected to other variables by some other means (shape_by_conn or a resolvable variable connects to it by its own shape_by_conn or copy_shape) then it'll just work anyway. Your comment also reminded me that I didn't have a test for when copy_shape is set to a non-existent variable, so I added that.

joanibal · 2020-09-09T23:55:03Z

Wow! This is a really nice implementation. Thank you for all the work that went into this.

I have just one suggestion. I think it would make it much easier for users to debug their models if they had some graphical representation of the size dependency graph. If there are unresolved variables the error message lists the variables that are an issue in alphabetical order, but doesn't say how they are connected.

With a picture to go along with the error message it is much easier to see the connections and understand why the test test_cycle_unresolved doesn't work.

This is something that could be done only if the model fails like this...

        if unresolved:
            unresolved = sorted(unresolved)
            #try to save a plot of the dependency graph for debuging
            msg = f"{self.msginfo}: Failed to resolve shapes for {unresolved}."
            try:
                import matplotlib.pyplot as plt
                nx.draw(graph, with_labels=True)
                plt.savefig('shape_dependency_graph.png')
                msg += " See shape_dependency_graph.png for more details"
            except ImportError:
                pass
            except err as e:
                raise err

            raise RuntimeError(msg)

However it is probably better to a have a utility function for this so users could verify that the dependencies are as expected even without an error. I'm not sure how to best implement that, but it sounds possible.

…ices with dynamic shape variables

naylor-b · 2020-09-10T19:48:17Z

@joanibal I added a 'view_dyn_shapes' command to the openmdao command line tool that will display the shape dependency graph using matplotlib, basically like you showed in your example. At some point when I have more time I'd like to replace the matplotlib stuff with a better graph viewing library, but for now, it's at least better than nothing.

swryan · 2020-09-14T14:49:25Z

openmdao/core/component.py

-    def add_input(self, name, val=1.0, shape=None, src_indices=None, flat_src_indices=None,
-                  units=None, desc='', tags=None):
+    def add_input(self, name, val=None, shape=None, src_indices=None, flat_src_indices=None,
+                  units=None, desc='', tags=None, shape_by_conn=False, copy_shape=None):


NOTE: changing the default value is a backward incompatibility?

I reverted this back to 1.0. So now, val is allowed to be set but it must be a scalar, and once the shape is known, that scalar is used to fill the array.

swryan · 2020-09-14T14:50:19Z

openmdao/core/component.py

+        declared here since this is called after all size/shape information is known for
+        all variables.
+        """
+        pass


NOTE: changing the API for where partials are declared is a backward incompatibility?

It's not backwardly incompatible. Doing it in the old way still works, although we recommend that people do it now in setup_partials for consistency sake.

swryan · 2020-09-14T15:13:32Z

openmdao/core/system.py

@@ -1346,6 +1364,7 @@ def _setup_global_shapes(self):
        """
        meta = self._var_allprocs_abs2meta
        loc_meta = self._var_abs2meta
+        myrank = self.comm.rank


swryan · 2020-09-14T15:21:46Z

openmdao/core/tests/test_dyn_sizing.py

+        self.add_output('out', shape=3)
+
+    def compute(self, inputs, outputs):
+        outputs['out'] *= self.comm.rank


this is strange, could you explain what's going on here (computing output based on itself)?

I updated this so the output now depends upon the input.

swryan · 2020-09-14T15:27:15Z

openmdao/docs/advanced_guide/derivs/partial_derivs_explicit.rst

@@ -22,7 +22,7 @@ Here is an example, based on the :ref:`Betz Limit Example <betz_limit_tutorial>`


 The calls to :code:`declare_partials` tell OpenMDAO which partial derivatives to expect.
-This is always done inside the :code:`setup` method.
+This should always be done inside the :code:`setup_partials` method.


should or must?
if should then more explanation is probably required...
if must then we do we need a deprecation or exception when it's done the old way?

I added more explanation about why setup_partials is the recommended way.

naylor-b added 27 commits August 25, 2020 11:23

tests passing. need more tests

a0da723

tests passing

2f48290

Merge branch 'master' of https://github.com/OpenMDAO/OpenMDAO into dy…

454ab04

…n_sizing

merged out pull_src_err branch. tests passing

c8daaf7

added cycle test

b0fc3f9

added more tests and reworked the algorithm to take advantage of the …

cd4acda

…graph

updated comment

8dd9861

Merge branch 'master' of https://github.com/OpenMDAO/OpenMDAO into dy…

b616a48

…n_shape

fixed test and added docs

1de2956

renamed dyn shape function to _setup_dynamic_shapes

e4a26ab

added resetup test

3a968e6

fix for resetup if vars created in __init__

f137b45

more testing. 1 failure

1bc86a3

Merge branch 'master' of https://github.com/OpenMDAO/OpenMDAO into dy…

a2f4d4d

…n_shape

commented out test

4570275

interim

1b80f08

tests passing

da900e4

cleanup

f9044b2

Merge branch 'master' of https://github.com/OpenMDAO/OpenMDAO into dy…

aeeb765

…n_shape

added mismatched dyn shape test and setup_partials hook

52bd7c5

tests passing. updated docs and moved declare_partials to setup_partials

c0bcf9e

more setup_partials conversions. added error message for shape mismat…

9e59d67

…ch during dyn shape determination

added test using dynamic sizing and setup_partials

605c1d6

added test for access to shape/size/value during configure for dynami…

c565c2a

…cally sized variables

Merge branch 'master' of https://github.com/OpenMDAO/OpenMDAO into dy…

8a75139

…n_shape

cleanup

b749870

cleanup

73cfd0d

naylor-b added the Pending POEM Acceptance Do not merge. This PR is attached to a POEM that is not yet accepted. label Sep 8, 2020

project-bot bot added this to In progress in OpenMDAO Dev [Read only] Sep 8, 2020

no-mpi fix

fdf431c

more tests

db6d6d2

OpenMDAO Dev [Read only] automation moved this from In progress to Reviewer approved Sep 9, 2020

Kenneth-T-Moore approved these changes Sep 9, 2020

View reviewed changes

added a test

41a9d80

naylor-b added 2 commits September 10, 2020 15:21

added openmdao command view_dyn_shapes, added error check for src_ind…

4877290

…ices with dynamic shape variables

fixed expected test results

137d857

naylor-b added 2 commits September 10, 2020 16:16

added check for empty graph

e0c8172

tweaked display of missing shapes in dyn shape plot

4308ab6

naylor-b removed the Pending POEM Acceptance Do not merge. This PR is attached to a POEM that is not yet accepted. label Sep 14, 2020

robfalck approved these changes Sep 14, 2020

View reviewed changes

swryan reviewed Sep 14, 2020

View reviewed changes

review fixes

cd543fa

swryan approved these changes Sep 14, 2020

View reviewed changes

swryan merged commit c03985f into OpenMDAO:master Sep 14, 2020

OpenMDAO Dev [Read only] automation moved this from Reviewer approved to Done Sep 14, 2020

naylor-b deleted the dyn_shape branch March 8, 2022 13:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implementation for POEM_022 - Determining variable shape at runtime based on connections #1671

Implementation for POEM_022 - Determining variable shape at runtime based on connections #1671

naylor-b commented Sep 8, 2020

Kenneth-T-Moore Sep 9, 2020

naylor-b Sep 9, 2020

joanibal commented Sep 9, 2020

naylor-b commented Sep 10, 2020

swryan Sep 14, 2020

naylor-b Sep 14, 2020

swryan Sep 14, 2020

naylor-b Sep 14, 2020

swryan Sep 14, 2020

naylor-b Sep 14, 2020

swryan Sep 14, 2020

naylor-b Sep 14, 2020

swryan Sep 14, 2020

naylor-b Sep 14, 2020

Implementation for POEM_022 - Determining variable shape at runtime based on connections #1671

Implementation for POEM_022 - Determining variable shape at runtime based on connections #1671

Conversation

naylor-b commented Sep 8, 2020

Summary

Backwards incompatibilities

New Dependencies

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joanibal commented Sep 9, 2020

naylor-b commented Sep 10, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment