Skip to content

New port classes - #1785

Open
dallan-keylogic wants to merge 9 commits into
IDAES:mainfrom
dallan-keylogic:new_port_classes
Open

New port classes#1785
dallan-keylogic wants to merge 9 commits into
IDAES:mainfrom
dallan-keylogic:new_port_classes

Conversation

@dallan-keylogic

Copy link
Copy Markdown
Contributor

Summary/Motivation:

A persistent problem in flowsheet visualization has been trying to determine whether a port corresponds to an inlet or outlet. The present "solution" has been to search for "inlet" or "outlet" in the port name. Here I added two subclasses of Port, so we can verify if something is an inlet, outlet, or neither using isinstance.

Changes proposed in this PR:

  • InletPort and OutletPort classes have been implemented
  • Unit models have been updated to use them

Legal Acknowledgement

By contributing to this software project, I agree to the following terms and conditions for my contribution:

  1. I agree my contributions are submitted under the license terms described in the LICENSE.txt file at the top level of this directory.
  2. I represent I am authorized to make the contributions and grant the license. If my employer has rights to intellectual property that includes these contributions, I represent that I have received permission to make contributions and grant the required license on behalf of that employer.

@dallan-keylogic
dallan-keylogic requested a review from jsiirola May 4, 2026 18:51
@codecov

codecov Bot commented May 4, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 70.83333% with 21 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.79%. Comparing base (c614556) to head (80693ca).
⚠️ Report is 19 commits behind head on main.

Files with missing lines Patch % Lines
idaes/core/base/ports.py 50.00% 16 Missing ⚠️
idaes/core/base/unit_model.py 71.42% 2 Missing ⚠️
..._models/soc_submodels/solid_oxide_module_simple.py 66.66% 1 Missing and 1 partial ⚠️
idaes/core/__init__.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1785      +/-   ##
==========================================
- Coverage   73.82%   73.79%   -0.03%     
==========================================
  Files         419      420       +1     
  Lines       66392    66428      +36     
  Branches    11171    11175       +4     
==========================================
+ Hits        49012    49022      +10     
- Misses      14840    14863      +23     
- Partials     2540     2543       +3     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@dallan-keylogic

Copy link
Copy Markdown
Contributor Author

The CodeCov failure is due to the same issue getting coverage for functions, classes and import statements and consequently will be ignored.

@andrewlee94 , let me know if you have any opinions about this change.

@andrewlee94

Copy link
Copy Markdown
Contributor

An alternative to consider that might be a bit more robust would be to give ports the following properties: is_inlet_port and is_outlet_port - these could point to a single underlying boolean attribute which can be used to decide which they are. Then you could use the properties instead of isinstance for classification, and would allow for possible duck-typing in future.

@dallan-keylogic

Copy link
Copy Markdown
Contributor Author

An alternative to consider that might be a bit more robust would be to give ports the following properties: is_inlet_port and is_outlet_port - these could point to a single underlying boolean attribute which can be used to decide which they are. Then you could use the properties instead of isinstance for classification, and would allow for possible duck-typing in future.

I thought about doing something like this, but 1) that would still require subtyping the full diamond structure of Port and 2) we would have to preface port_obj.is_inlet_port() with `hasattr(port_obj, "is_inlet_port"), because users need to switch over their models to using the new subclass. Nevertheless, those hoops might be worthwhile for full duck typing.

The other method I thought about using was to look for ports in an arc as the "destination" field and treating those as inlets and ports in "source" field of an arc to treat that as an outlet. However, that method is also vulnerable to user error, and users also like to fix boundary conditions directly on inlet ports without using a Feed block. Also, the subclass of InletPort and OutletPort could be modified to raise an exception if the user tries to add them to an arc in an unexpected way (like making an InletPort a source or an OutletPort a destination).

@blnicho

blnicho commented May 6, 2026

Copy link
Copy Markdown
Member

@dallan-keylogic if inlet/outlet information is mostly captured in the Arc then I would push for using that rather than making custom port classes with duplicate information. Could you elaborate on the edge cases or issues that you mentioned in your previous comment that lead you to thinking this is not a viable path?

@dallan-keylogic

Copy link
Copy Markdown
Contributor Author

@dallan-keylogic if inlet/outlet information is mostly captured in the Arc then I would push for using that rather than making custom port classes with duplicate information. Could you elaborate on the edge cases or issues that you mentioned in your previous comment that lead you to thinking this is not a viable path?

The problem is that there are multiple places where inlet/outlet information are added, but they do not necessarily agree with each other. The first is in the source and destination fields of Arc objects. However, if a port is an inlet not just for a block but for the entire flowsheet, it may not be attached to an Arc. The Feed block was supposed to fix this, but that requires user discipline. @dangunter can attest to the fact that users often can't be relied on to add Feed and Product blocks where appropriate.

The second place that information is located is in the unit model's initialization method. Almost every initialization method ends up fixing variables on inlet ports. However, there's no class features that force the user to do this.

The last resort for determining what's an inlet and what's an outlet is the port's name. In multiple places in the IDAES codebase, iterating over Port objects and looking for inlet in the name is used to try to determine what directionality a port has.

Most of these issues can be handled by experienced users, but new users can find them tricky. One of the motivations for this PR came from watertap-org/watertap#1757. It would be nice to tell the user which ports are inlets and which ports are outlets as part of a similar method in IDAES.

@ksbeattie
ksbeattie requested review from CopyDemon and dangunter and removed request for MarcusHolly and agarciadiego July 2, 2026 18:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

diagnostics Priority:High High Priority Issue or PR ui

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants