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

Redesign the Pyomo Connector component #375

Closed
7 tasks done
jsiirola opened this issue Mar 1, 2018 · 8 comments
Closed
7 tasks done

Redesign the Pyomo Connector component #375

jsiirola opened this issue Mar 1, 2018 · 8 comments

Comments

@jsiirola
Copy link
Member

jsiirola commented Mar 1, 2018

We have started accumulating a number of documented issues with the current implementation of Connector components. This issue is an umbrella issue to track them, as well as centralize discussion around a redesign of the Connector component.

Known Connector issues:

@eslickj
Copy link
Contributor

eslickj commented Mar 16, 2018

It would be very useful for the IDAES project if there was some way to store the direction of a connector. Maybe if the connection constraints could have some deterministic ordering of connectors in the expression. The direction doesn't really mean anything for Pyomo, but in displaying process flowsheets and sequential modular initialization it is important.

@qtothec
Copy link
Contributor

qtothec commented Mar 16, 2018

We could have an IDAES subclass of Connector that stores the directionality information?

@eslickj
Copy link
Contributor

eslickj commented Mar 16, 2018

If we had directional connectors that would also solve the problem. Just define them as in or out. I kind of prefer the direction to be attached to the constraint, but I don't have a solid reason for that. If labeling connectors in/out is best way to handle it, I can create a IDAES issue and we can cross my comment off the list.

@blnicho blnicho added to do and removed to do labels Apr 3, 2018
@jsiirola jsiirola moved this from To do to Medium Priority in pyomo.core Priorities Apr 10, 2018
@qtothec qtothec added this to To Do [triage] in IDAES Priorities via automation May 31, 2018
@qtothec qtothec moved this from To Do [triage] to High priority in IDAES Priorities May 31, 2018
@gseastream
Copy link
Contributor

gseastream commented Jul 18, 2018

I believe most of these issues will be solved by the new pyomo network Port/Arc package in #583. I would like to update this collection of issues and close what we can when the merge happens, but I also want to make sure these issues are actually solved, so please reply if you think some of these will not be resolved by the new system.

#91, #96 will definitely be solved because the Arc expander only expands active Arcs and then deactivates them afterwards

#107 will definitely be solved because the new transformation does not encounter this bug, for which I opened #618

#128 I'm pretty sure will be solved if I understood @blnicho correctly, since the new constraints are indexed by the same sets as the variables in the ports

#227 I'm not sure. I think this will be fixed by @jsiirola's slice thing he's working on?

#297 I'm not sure. The classes and methods are pretty well documented in the code, but I assume someone wants to me to put an rst file on sphinx so I guess I can throw one together

#523 will probably be fixed, or it'll at least be addressed since the new transformation has performance improvements and advantages over the old Connector expander

#376 does not appear to be relevant to connectors so I'll assume that was a typo

#290 has several issues in it:

  1. Arcs replace the old way of defining constraints with connectors, and Arcs have their own ctype so they can be found instantly
  2. All expanded constraints (except for some of the extensive ones) are placed on a block and the arc maintains a reference to that block, so you can access the collection of all constraints with stream1.expanded_block and deactivate the whole thing or whatever you want
  3. You can access each constraint individually from this expanded block, and they are all named exactly by the name of the port member followed by "_equality", so if your port has something named "flow" there will be a constraint called flow_equality on the expanded block. You can access this via stream1.expanded_block.flow_equality, or you can even do stream1.flow_equality since I modified the __getattr__ method to look on the expanded block for things.
  4. See slice comment above
  5. As stated in the discussion, you can access variables from the port object via the vars dictionary attribute, but now with ports you can also do port1.flow and like the __getattr__ method above it will look in the vars dictionary for you. You can also call port1.fix() and it will fix every variable in the port at its current value.

@eslickj
Copy link
Contributor

eslickj commented Jul 19, 2018

Just pinging @andrewlee94. He would be interested, and doesn't appear to be a participant in this issue yet.

@gseastream
Copy link
Contributor

Per my comment above, with the recent merge of Pyomo network, #91, #96, #107, #523, and maybe others can be closed now. I would also still appreciate insight on some of the other issues listed above as to whether or not they are now solved.

@qtothec
Copy link
Contributor

qtothec commented Jul 25, 2018

Well if you want to test them and see if they're still issues, then we can close as appropriate.

@blnicho
Copy link
Member

blnicho commented Aug 27, 2018

Completed with #655

@blnicho blnicho closed this as completed Aug 27, 2018
IDAES Priorities automation moved this from High priority to Done Aug 27, 2018
pyomo.core Priorities automation moved this from Medium Priority to Done Aug 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

No branches or pull requests

5 participants