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

Generated code: removing creation of IndepVarComp #59

Closed
christophe-david opened this issue May 28, 2021 · 6 comments · Fixed by #79
Closed

Generated code: removing creation of IndepVarComp #59

christophe-david opened this issue May 28, 2021 · 6 comments · Fixed by #79

Comments

@christophe-david
Copy link

In the setup() of the problem.model, WhatsOpt creates an IndepVarComp instance to fill problem inputs.

Such IndepVarComp is no more needed since OpenMDAO 3.2.

And in some cases (that indeed do not fully respect the convention to have all variables promoted), WhatsOpt may wrongly identify a variable as an input, needed in the IVC.

Therefore I kindly suggest to remove this IndepVarComp from the generated code.

@relf
Copy link
Collaborator

relf commented May 28, 2021

A bit more complicated than just removing the code. Indeed IndepVarComp as generated by WhatsOpt get the right variable names (x, z, etc.) while OpenMDAO assign default names (v0, v1, etc.).

Brutal removal would break wop push run_analysis.py and wop upload run_parameter_init.py capability to upload parameters values set by the user in the run_parameters_init.py. Should revisit the way wop retrieve these values before changing the code generation.

@christophe-david
Copy link
Author

I understand.

Then I will have a deeper look at what problem I have with the generated IVC in my use case, and will report to you.

@relf
Copy link
Collaborator

relf commented May 31, 2021

I wait for your feedback. Anyway, I think I will try to get rid of IndepVarComp in the generated code.

@christophe-david
Copy link
Author

Ok, I got to the root of my issue. It is finally rather simple: I have connections between 2 promoted variables.

I know this is not in the assumptions of WhatsOpt, but my use case really needs it, as I have some generic component that will do the connection or not, depending on some option.

Whether it is a legit issue or not, I put down here the snippet I built to understand the problem. Output b1 and input b2 are connected, so that b2 does not need to be fed by an IVC, but wop pull will generate code where the IVC provides b2.

import openmdao.api as om


class Disc1(om.ExplicitComponent):
    def setup(self):
        self.add_input("a")
        self.add_output("b1")


class Disc2(om.ExplicitComponent):
    def setup(self):
        self.add_input("b2")
        self.add_output("c")


class MyGroup(om.Group):
    def setup(self):
        self.add_subsystem("disc1", Disc1(), promotes=["*"])
        self.add_subsystem("disc2", Disc2(), promotes=["*"])

        self.connect(src_name="b1", tgt_name="b2")


class Issue59(om.Group):
    def setup(self):
        self.add_subsystem("group", MyGroup(), promotes=["*"])


if __name__ == "__main__":
    pb = om.Problem(Issue59())
    pb.setup()
    pb.run_model()

@relf
Copy link
Collaborator

relf commented Jun 10, 2021

Thanks for the thorough reporting. Indeed this is out of the current WhatsOpt philosophy regarding variable/connection handling.

In WhatsOpt variables are created out of named connections, translating in OpenMDAO has automated connection between variables of the same name, excluding the usage of OpenMDAO connect() capability between variables of different names.

So when you pull/push such code in WhatsOpt you end up with unwanted connections to/from the driver and a missing the connection between b1 and b2.

If I get rid of IndepVarComp generation, the pulled code would be at least usable (provided you amend it by adding the b1-b2 connection by overriding the analysis setup method).

@relf
Copy link
Collaborator

relf commented Jun 15, 2021

@christophe-david. I've updated WhatsOpt and published wop 1.14.0. You can test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants