fast-aircraft-design / FAST-OAD

FAST-OAD: An open source framework for rapid Overall Aircraft Design
GNU General Public License v3.0
53 stars 26 forks source link

Connected promoted inputs are considered as global inputs #334

Closed ScottDelbecq closed 3 years ago

ScottDelbecq commented 3 years ago

There is an problem when determining wether if a variable is a global input of the problem when using the connect() mechanism with the promotion mechanism. In the example below, y1 is an output of dis1 and an input of functions which are both connected. Hence, y1.is_input should be False.

import numpy as np
import openmdao.api as om
from fastoad.openmdao.variables import VariableList

class Disc1(om.ExplicitComponent):
    """ An OpenMDAO component to encapsulate Disc1 discipline """

    def setup(self):
        self.add_input("x", val=np.nan, desc="")  # NaN as default for testing connexion check
        self.add_input("z", val=[5, 2], desc="", units="m**2")  # for testing non-None units
        self.add_input("y2", val=1.0, desc="")

        self.add_output("y1", val=1.0, desc="")
        self.declare_partials("*", "*", method="fd")

    def compute(self, inputs, outputs, discrete_inputs=None, discrete_outputs=None):
        """
        Evaluates the equation
        y1 = z1**2 + z2 + x1 - 0.2*y2
        """
        z1 = inputs["z"][0]
        z2 = inputs["z"][1]
        x1 = inputs["x"]
        y2 = inputs["y2"]

        outputs["y1"] = z1 ** 2 + z2 + x1 - 0.2 * y2

class Disc2(om.ExplicitComponent):
    """ An OpenMDAO component to encapsulate Disc2 discipline """

    def setup(self):
        self.add_input("z", val=[5, 2], desc="", units="m**2")  # for testing non-None units
        self.add_input("y1", val=1.0, desc="")

        self.add_output("y2", val=1.0, desc="")
        self.declare_partials("*", "*", method="fd")

    def compute(self, inputs, outputs, discrete_inputs=None, discrete_outputs=None):
        """
        Evaluates the equation
        y2 = y1**(.5) + z1 + z2
        """

        z1 = inputs["z"][0]
        z2 = inputs["z"][1]
        y1 = inputs["y1"]

        # Note: this may cause some issues. However, y1 is constrained to be
        # above 3.16, so lets just let it converge, and the optimizer will
        # throw it out
        if y1.real < 0.0:
            y1 *= -1

        outputs["y2"] = y1 ** 0.5 + z1 + z2

class Functions(om.ExplicitComponent):
    """ An OpenMDAO component to encapsulate Functions discipline """

    def setup(self):
        self.add_input("x", val=2, desc="")
        self.add_input(
            "z", val=[np.nan, np.nan], desc="", units="m**2"
        )  # NaN as default for testing connexion check
        self.add_input("y1", val=1.0, desc="")
        self.add_input("y2", val=1.0, desc="")

        self.add_output("f", val=1.0, desc="")

        self.add_output("g1", val=1.0, desc="")

        self.add_output("g2", val=1.0, desc="")
        self.declare_partials("*", "*", method="fd")

    def compute(self, inputs, outputs, discrete_inputs=None, discrete_outputs=None):
        """ Functions computation """

        z2 = inputs["z"][1]
        x1 = inputs["x"]
        y1 = inputs["y1"]
        y2 = inputs["y2"]

        outputs["f"] = x1 ** 2 + z2 + y1 + exp(-y2)
        outputs["g1"] = 3.16 - y1
        outputs["g2"] = y2 - 24.0

group = om.Group()
group.add_subsystem("disc1", Disc1(),  promotes=["x", "z"])
group.add_subsystem("disc2", Disc2(),  promotes=["z"])
group.add_subsystem("functions", Functions(),  promotes=["*"])

group.connect("disc1.y1", "disc2.y1")
group.connect("disc2.y2", "disc1.y2")
group.connect("disc1.y1", "y1")
group.connect("disc2.y2", "y2")

problem = om.Problem(group)
problem.setup()

vars = VariableList.from_problem(problem, use_initial_values=False, get_promoted_names=True)

# Should be False
print(vars["y1"].is_input)
christophe-david commented 3 years ago

Ok, this is a corner case that VariableList.from_problem() does not handle, but how is it a problem for FAST-OAD? I mean, in a FAST-OAD computation, your 3 components would be FAST-OAD modules, and their inputs and outputs would be all promoted. Can we have the same problem in a real case usage of FAST-OAD?

ScottDelbecq commented 3 years ago

This issue will not impact the computation and thus the problem results. However, I find it confusing to have an output in the generated input file. The only place where this occurs in FAST-OAD is here when the use_xfoil option is set to True. This leads to have data:aerodynamics:aircraft:landing:CL_max_clean_2D in the generated input file despite that it is then overwritten by the computation. This will occur each time we want to connect a non promoted and a promoted variable. I can't tell if many users will do this.

christophe-david commented 3 years ago

Actually, you should find that this variable is no more in input file with current master, because it was due to bug #313, which has just been fixed.

And since FAST-OAD always promotes all variables of its modules, the only place where a user can choose to connect a non promoted and a promoted variable, is inside his own components. And in that case, I am not really sure that it could have the effect you describe on the whole problem.

My point is that VariableList.from_problem() is already (too?) complicated. And if you have to add code in it to manage this corner case, I am afraid that refactoring/splitting this method might be necessary to keep it readable. And of course, you'd have to set up also a proper unit test to go with your changes.

I am not against doing that, but I just want to be sure it is worth it.