diff --git a/CHANGELOG.md b/CHANGELOG.md index 3745c48..ab5e58d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -100,6 +100,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 function against the residual and the variable against the input or output, matching how `SetVariableShapes` already indexes. An unknown name now raises `PhiloteValidationError` instead of `KeyError` (#79). +- `OpenMdaoSubProblem.compute_partials` built the `of` and `wrt` lists for + `compute_totals` with one entry per declared partial, so an output with + several inputs (or an input feeding several outputs) was named repeatedly. + Both lists are now deduplicated while preserving declaration order, which + removes redundant work inside `compute_totals` on every gradient call. + Results were already correct, since the totals are indexed by the + `(of, wrt)` pair rather than by position (#80). ### Documentation & Infrastructure diff --git a/philote_mdo/openmdao/group.py b/philote_mdo/openmdao/group.py index f27f9d9..e92fd73 100644 --- a/philote_mdo/openmdao/group.py +++ b/philote_mdo/openmdao/group.py @@ -368,11 +368,11 @@ def compute_partials(self, inputs, partials): self._prob.run_model() # get the list of functions and variables for the compute_totals call - func = [] - var = [] - for val in self._partials_map.values(): - func += [val[0]] - var += [val[1]] + # the partials map is keyed on (output, input) pairs, so a name can + # appear in several entries; dict.fromkeys drops the repeats while + # preserving the declaration order + func = list(dict.fromkeys(val[0] for val in self._partials_map.values())) + var = list(dict.fromkeys(val[1] for val in self._partials_map.values())) totals = self._prob.compute_totals(of=func, wrt=var) diff --git a/tests/test_openmdao_group.py b/tests/test_openmdao_group.py index c78b0fa..48e8a85 100644 --- a/tests/test_openmdao_group.py +++ b/tests/test_openmdao_group.py @@ -297,6 +297,61 @@ def test_declare_subproblem_partial_unmapped_input_raises(self): with self.assertRaises(PhiloteValidationError): subprob.declare_subproblem_partial("y", "unmapped_x") + def test_compute_partials_deduplicates_totals_arguments(self): + """ + Test that compute_totals is called without repeated of/wrt entries. + """ + subprob = OpenMdaoSubProblem() + + group = om.Group() + group.add_subsystem( + "comp", om.ExecComp(["y1 = 2*x1 + 3*x2", "y2 = 4*x1"]), promotes=["*"] + ) + subprob.add_group(group) + + subprob.add_mapped_input("local_x1", "x1") + subprob.add_mapped_input("local_x2", "x2") + subprob.add_mapped_output("local_y1", "y1") + subprob.add_mapped_output("local_y2", "y2") + + # y1 depends on both inputs and x1 feeds both outputs, so a naive + # build of the argument lists repeats "y1" and "x1" + subprob.declare_subproblem_partial("local_y1", "local_x1") + subprob.declare_subproblem_partial("local_y1", "local_x2") + subprob.declare_subproblem_partial("local_y2", "local_x1") + + subprob.setup() + + calls = [] + original = subprob._prob.compute_totals + + def spy(of, wrt): + calls.append((of, wrt)) + return original(of=of, wrt=wrt) + + subprob._prob.compute_totals = spy + + inputs = {"local_x1": np.array([3.0]), "local_x2": np.array([4.0])} + partials = { + ("local_y1", "local_x1"): np.array([0.0]), + ("local_y1", "local_x2"): np.array([0.0]), + ("local_y2", "local_x1"): np.array([0.0]), + } + + subprob.compute_partials(inputs, partials) + + self.assertEqual(len(calls), 1) + func, var = calls[0] + self.assertEqual(func, list(dict.fromkeys(func))) + self.assertEqual(var, list(dict.fromkeys(var))) + self.assertEqual(sorted(func), ["y1", "y2"]) + self.assertEqual(sorted(var), ["x1", "x2"]) + + # the derivatives are still assembled correctly + self.assertAlmostEqual(partials[("local_y1", "local_x1")][0], 2.0, places=6) + self.assertAlmostEqual(partials[("local_y1", "local_x2")][0], 3.0, places=6) + self.assertAlmostEqual(partials[("local_y2", "local_x1")][0], 4.0, places=6) + if __name__ == "__main__": unittest.main(verbosity=2)