Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Connected promoted inputs are considered as global inputs #334

Closed
ScottDelbecq opened this issue May 11, 2021 · 3 comments · Fixed by #341
Closed

Connected promoted inputs are considered as global inputs #334

ScottDelbecq opened this issue May 11, 2021 · 3 comments · Fixed by #341
Assignees
Labels
bug Something isn't working

Comments

@ScottDelbecq
Copy link
Contributor

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)
@ScottDelbecq ScottDelbecq added the bug Something isn't working label May 11, 2021
@ScottDelbecq ScottDelbecq self-assigned this May 11, 2021
@christophe-david
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants