Merge pull request #434 from fast-aircraft-design/issue-431_pickle-pr…

…oblem

Fixed crash when using Newton solver or case recorders

src/fastoad/openmdao/tests/data/ref_inputs.xml

@@ -1,4 +1,19 @@
+<!--
+  ~ This file is part of FAST-OAD : A framework for rapid Overall Aircraft Design
+  ~ Copyright (C) 2022 ONERA & ISAE-SUPAERO
+  ~ FAST is free software: you can redistribute it and/or modify
+  ~ it under the terms of the GNU General Public License as published by
+  ~ the Free Software Foundation, either version 3 of the License, or
+  ~ (at your option) any later version.
+  ~ This program is distributed in the hope that it will be useful,
+  ~ but WITHOUT ANY WARRANTY; without even the implied warranty of
+  ~ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+  ~ GNU General Public License for more details.
+  ~ You should have received a copy of the GNU General Public License
+  ~ along with this program.  If not, see <https://www.gnu.org/licenses/>.
+  -->
+
 <FASTOAD_model>
-  <x is_input="True">2000.0</x>
-  <z units="m**2" is_input="True">[5000.0, 2000.0]</z>
+  <x is_input="True">1.0</x>
+  <z units="m**2" is_input="True">[4.0, 3.0]</z>
 </FASTOAD_model>

src/fastoad/openmdao/tests/openmdao_sellar_example/sellar.py

@@ -16,7 +16,7 @@
 import abc
 from typing import Type
 
-from openmdao.api import Group, IndepVarComp, NonlinearBlockGS
+import openmdao.api as om
 
 from .disc1 import Disc1
 from .disc2 import Disc2
@@ -62,7 +62,7 @@ def create_functions():
         return Functions()
 
 
-class Sellar(Group):
+class Sellar(om.Group):
     """An OpenMDAO base component to encapsulate Sellar MDA"""
 
     def __init__(self, sellar_factory: Type[ISellarFactory] = StandardSellarFactory, **kwargs):
@@ -75,15 +75,14 @@ def __init__(self, sellar_factory: Type[ISellarFactory] = StandardSellarFactory,
         super(Sellar, self).__init__(**kwargs)
 
         self._sellar_factory = sellar_factory
-        self.nonlinear_solver = NonlinearBlockGS()
-        self.nonlinear_solver.options["atol"] = 1.0e-10
-        self.nonlinear_solver.options["rtol"] = 1.0e-10
-        self.nonlinear_solver.options["maxiter"] = 10
-        self.nonlinear_solver.options["err_on_non_converge"] = True
-        self.nonlinear_solver.options["iprint"] = 1
+
+        # This combination of solvers is specifically chosen because it creates some
+        # non-pickle-able object that will cause problems in case of deepcopy (see issue #431)
+        self.nonlinear_solver = om.NewtonSolver(solve_subsystems=False, maxiter=50)
+        self.linear_solver = om.DirectSolver()
 
     def setup(self):
-        indeps = self.add_subsystem("indeps", IndepVarComp(), promotes=["*"])
+        indeps = self.add_subsystem("indeps", om.IndepVarComp(), promotes=["*"])
         indeps.add_output("x", 2)
         self.add_subsystem("Disc1", self._sellar_factory.create_disc1(), promotes=["x", "z", "y2"])
         self.add_subsystem("Disc2", self._sellar_factory.create_disc2(), promotes=["z", "y2"])

src/fastoad/openmdao/tests/test_problem.py

@@ -12,6 +12,7 @@
 #  along with this program.  If not, see <https://www.gnu.org/licenses/>.
 
 import os.path as pth
+from os import makedirs
 from shutil import rmtree
 
 import numpy as np
@@ -34,6 +35,7 @@
 @pytest.fixture(scope="module")
 def cleanup():
     rmtree(RESULTS_FOLDER_PATH, ignore_errors=True)
+    makedirs(RESULTS_FOLDER_PATH)
 
 
 def test_write_outputs():
@@ -91,8 +93,9 @@ def test_problem_read_inputs_after_setup(cleanup):
     problem.read_inputs()
 
     problem.run_model()
-    assert problem.get_val(name="x") == [2000.0]
-    assert_allclose(problem.get_val(name="z", units="m**2"), [5000, 2000.0])
+    assert_allclose(problem.get_val(name="x"), 1.0)
+    assert_allclose(problem.get_val(name="z", units="m**2"), [4.0, 3.0])
+    assert_allclose(problem["f"], 21.7572, atol=1.0e-4)
 
 
 def test_problem_read_inputs_before_setup(cleanup):
@@ -105,9 +108,33 @@ def test_problem_read_inputs_before_setup(cleanup):
 
     problem.read_inputs()
     problem.setup()
+    problem.run_model()
+
+    assert_allclose(problem.get_val(name="x"), 1.0)
+    assert_allclose(problem.get_val(name="z", units="m**2"), [4.0, 3.0])
+    assert_allclose(problem["f"], 21.7572, atol=1.0e-4)
+
+
+def test_problem_with_case_recorder(cleanup):
+    """Tests what happens when using a case recorder"""
+    # Adding a case recorder may cause a crash in case of deepcopy.
+
+    problem = FASTOADProblem()
+    sellar = Sellar()
+    sellar.nonlinear_solver = om.NonlinearBlockGS()  # Solver that is compatible with deepcopy
+    sellar.add_recorder(om.SqliteRecorder(pth.join(RESULTS_FOLDER_PATH, "cases.sql")))
+
+    problem.model.add_subsystem("sellar", sellar, promotes=["*"])
+
+    problem.input_file_path = pth.join(DATA_FOLDER_PATH, "ref_inputs.xml")
+
+    problem.setup()
+    problem.read_inputs()
+    problem.run_model()
 
-    assert problem.get_val(name="x") == [2000.0]
-    assert np.all(problem.get_val(name="z", units="m**2") == [5000.0, 2000.0])
+    assert_allclose(problem.get_val(name="x"), 1.0)
+    assert_allclose(problem.get_val(name="z", units="m**2"), [4.0, 3.0])
+    assert_allclose(problem["f"], 21.7572, atol=1.0e-4)
 
 
 def test_problem_read_inputs_with_nan_inputs(cleanup):

src/fastoad/openmdao/tests/test_variables.py

@@ -277,43 +277,19 @@ def test_get_variables_from_problem_sellar_with_promotion_without_computation():
         Variable(name="functions.g2", val=np.array([1.0]), units=None, is_input=False),
         Variable(name="functions.f", val=np.array([1.0]), units=None, is_input=False),
     ]
-    # Even without computation, not using initial value will unify values of connected variables.
-    expected_vars_non_promoted_computed = [
-        Variable(name="indeps.x", val=np.array([1.0]), units="Pa", is_input=True),
-        Variable(name="indeps.z", val=np.array([5.0, 2.0]), units="m**2", is_input=True),
-        Variable(name="disc1.x", val=np.array([1.0]), units=None, is_input=True, desc="input x"),
-        Variable(name="disc1.z", val=np.array([5.0, 2.0]), units="m**2", is_input=True),
-        Variable(
-            name="disc1.y1", val=np.array([1.0]), units=None, is_input=False, desc="variable y1"
-        ),
-        Variable(
-            name="disc1.y2", val=np.array([1.0]), units=None, is_input=True, desc="variable y2"
-        ),
-        Variable(
-            name="disc2.z", val=np.array([5.0, 2.0]), units="m**2", is_input=True, desc="variable z"
-        ),
-        Variable(name="disc2.y1", val=np.array([1.0]), units=None, is_input=True),
-        Variable(name="disc2.y2", val=np.array([1.0]), units=None, is_input=False),
-        Variable(name="functions.x", val=np.array([1.0]), units=None, is_input=True),
-        Variable(name="functions.z", val=np.array([5.0, 2.0]), units="m**2", is_input=True),
-        Variable(name="functions.y1", val=np.array([1.0]), units=None, is_input=True),
-        Variable(name="functions.y2", val=np.array([1.0]), units=None, is_input=True),
-        Variable(name="functions.g1", val=np.array([1.0]), units=None, is_input=False),
-        Variable(name="functions.g2", val=np.array([1.0]), units=None, is_input=False),
-        Variable(name="functions.f", val=np.array([1.0]), units=None, is_input=False),
-    ]
 
     vars = VariableList.from_problem(problem, use_initial_values=True, get_promoted_names=True)
     _compare_variable_lists(vars, expected_vars_promoted)
 
     vars = VariableList.from_problem(problem, use_initial_values=True, get_promoted_names=False)
     _compare_variable_lists(vars, expected_vars_non_promoted)
 
+    # use_initial_values=False should have no effect while problem has not been run
     vars = VariableList.from_problem(problem, use_initial_values=False, get_promoted_names=True)
     _compare_variable_lists(vars, expected_vars_promoted)
 
     vars = VariableList.from_problem(problem, use_initial_values=False, get_promoted_names=False)
-    _compare_variable_lists(vars, expected_vars_non_promoted_computed)
+    _compare_variable_lists(vars, expected_vars_non_promoted)
 
 
 def test_get_variables_from_problem_sellar_with_promotion_with_computation():
@@ -473,32 +449,6 @@ def test_get_variables_from_problem_sellar_without_promotion_without_computation
         Variable(name="functions.f", val=np.array([1.0]), units=None, is_input=False),
     ]
 
-    # Even without computation, not using initial value will unify values of connected variables.
-    expected_vars_computed = [
-        Variable(name="indeps.x", val=np.array([1.0]), units="Pa", is_input=True),
-        Variable(name="indeps.z", val=np.array([5.0, 2.0]), units="m**2", is_input=True),
-        Variable(name="disc1.x", val=np.array([1.0]), units=None, is_input=True, desc="input x"),
-        Variable(name="disc1.z", val=np.array([5.0, 2.0]), units="m**2", is_input=True),
-        Variable(
-            name="disc1.y1", val=np.array([1.0]), units=None, is_input=False, desc="variable y1"
-        ),
-        Variable(
-            name="disc1.y2", val=np.array([1.0]), units=None, is_input=True, desc="variable y2"
-        ),
-        Variable(
-            name="disc2.z", val=np.array([5.0, 2.0]), units="m**2", is_input=True, desc="variable z"
-        ),
-        Variable(name="disc2.y1", val=np.array([1.0]), units=None, is_input=True),
-        Variable(name="disc2.y2", val=np.array([1.0]), units=None, is_input=False),
-        Variable(name="functions.x", val=np.array([1.0]), units=None, is_input=True),
-        Variable(name="functions.z", val=np.array([5.0, 2.0]), units="m**2", is_input=True),
-        Variable(name="functions.y1", val=np.array([1.0]), units=None, is_input=True),
-        Variable(name="functions.y2", val=np.array([1.0]), units=None, is_input=True),
-        Variable(name="functions.g1", val=np.array([1.0]), units=None, is_input=False),
-        Variable(name="functions.g2", val=np.array([1.0]), units=None, is_input=False),
-        Variable(name="functions.f", val=np.array([1.0]), units=None, is_input=False),
-    ]
-
     vars = VariableList.from_problem(
         problem, use_initial_values=True, get_promoted_names=True, promoted_only=True
     )
@@ -514,15 +464,21 @@ def test_get_variables_from_problem_sellar_without_promotion_without_computation
     )
     _compare_variable_lists(vars, expected_vars_initial)
 
+    # use_initial_values=False should have no effect while problem has not been run
+    vars = VariableList.from_problem(
+        problem, use_initial_values=False, get_promoted_names=True, promoted_only=True
+    )
+    _compare_variable_lists(vars, [])
+
     vars = VariableList.from_problem(
         problem, use_initial_values=False, get_promoted_names=True, promoted_only=False
     )
-    _compare_variable_lists(vars, expected_vars_computed)
+    _compare_variable_lists(vars, expected_vars_initial)
 
     vars = VariableList.from_problem(
         problem, use_initial_values=False, get_promoted_names=False, promoted_only=False
     )
-    _compare_variable_lists(vars, expected_vars_computed)
+    _compare_variable_lists(vars, expected_vars_initial)
 
 
 def test_get_variables_from_problem_sellar_without_promotion_with_computation():

src/fastoad/openmdao/variables/variable_list.py

@@ -252,16 +252,17 @@ def from_problem(
             Variables from _auto_ivc are ignored.
 
         :param problem: OpenMDAO Problem instance to inspect
-        :param use_initial_values: if True, returned instance will contain values before computation
+        :param use_initial_values: if True, or if problem has not been run, returned instance will
+                                   contain values before computation
         :param get_promoted_names: if True, promoted names will be returned instead of absolute ones
                                    (if no promotion, absolute name will be returned)
         :param promoted_only: if True, only promoted variable names will be returned
         :param io_status: to choose with type of variable we return ("all", "inputs, "inputs")
         :return: VariableList instance
         """
 
-        problem = deepcopy(problem)
         if not problem._metadata or problem._metadata["setup_status"] < _SetupStatus.POST_SETUP:
+            problem = deepcopy(problem)
             problem.setup()
 
         # Get inputs and outputs
@@ -348,9 +349,11 @@ def from_problem(
         input_vars = VariableList.from_dict(final_inputs)
         output_vars = VariableList.from_dict(final_outputs)
 
-        # Use computed value instead of initial ones, if asked for
-        for variable in input_vars + output_vars:
-            if not use_initial_values:
+        # Use computed value instead of initial ones, if asked for, and if problem has been run.
+        # Note: using problem.get_val() if problem has not been run may lead to unexpected
+        # behaviour when actually running the problem.
+        if not use_initial_values and problem.model.iter_count > 0:
+            for variable in input_vars + output_vars:
                 try:
                     # Maybe useless, but we force units to ensure it is consistent
                     variable.value = problem.get_val(variable.name, units=variable.units)