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

Constants for connector coordinates not properly handled by Modelica tools #1794

Closed
AntoineGautier opened this issue Sep 20, 2023 · 4 comments
Assignees
Labels

Comments

@AntoineGautier
Copy link
Contributor

AntoineGautier commented Sep 20, 2023

The bug results from #1782 which introduced some constants to parameterize the placement of outside connectors in interface classes within Fluid.
Dymola (2023x Refresh1 on Linux and Windows)

  • does not properly handle these constants when extending such interface classes, as illustrated in the example below,
  • (even worse!) modifies existing annotations when opening models that derive from these classes: for instance, open IBPSA.Fluid.Chillers.BaseClasses.Carnot and notice how these lines
    connect(port_a2, eva.port_a)
    annotation (Line(points={{100,-60},{56,-60},{10,-60}}, color={0,127,255}));
    are modified into
    connect(port_a2, eva.port_a)
      annotation (Line(points={{0,0},{10,0},{10,-60}},       color={0,127,255}));
    

OMEdit (OpenModelica 1.21.0 on Linux)

  • does not properly handle these constants when extending such interface classes, as illustrated in the example below,
  • but does not modify existing models like Dymola does.

I have opened a ticket at Dassault Systemes: SRF01088076 (status: wait for correction delivery).
However, I see no immediate workaround (changing these constants to parameters and/or making them public has no effect), and I suggest that we readily revert the changes from #1782 as Dymola's behavior is likely to have severe consequences.

@mwetter or @FWuellhorst Can you reproduce what I noticed on your system?


Example illustrating the issue: Connecting Component.port_a to Component.sub.port_a from the diagram view of Dymola yields an incorrect graphical annotation.

package Test
model Base
  Modelica.Fluid.Interfaces.FluidPort_a port_a
    "Fluid connector a (positive design flow direction is from port_a to port_b)"
    annotation (Placement(transformation(extent={{port_a_x-10,port_a_y-10},{port_a_x+10,port_a_y+10}})));
  Modelica.Fluid.Interfaces.FluidPort_b port_b
    "Fluid connector b (positive design flow direction is from port_a to port_b)"
    annotation (Placement(transformation(extent={{port_b_x-10,port_b_y-10},{port_b_x+10,port_b_y+10}})));
  protected 
  constant Integer port_a_x = -100
    "x-coordinate of port_a center";
  constant Integer port_a_y = 0
    "y-coordinate of port_a center";
  constant Integer port_b_x = 100
    "x-coordinate of port_b center";
  constant Integer port_b_y = 0
    "y-coordinate of port_b center";
end Base;

model SubComponent
  extends Base;
end SubComponent;

model Component
  extends Base;
  SubComponent sub
    annotation (Placement(transformation(extent={{-10,-10},{10,10}})));
equation 
  connect(port_a, sub.port_a)
    annotation (Line(points={{0,0},{-6,0},{-6,0},{-10,0}}, color={0,127,255}));
end Component;
end Test;
@AntoineGautier AntoineGautier self-assigned this Sep 20, 2023
@FWuellhorst
Copy link
Contributor

Yes, I can reproduce this. Good example of how even two reviewers can miss something with such an impact.
I still like some of the changes of #1782. Maybe just revert the examples and the constants?

@mwetter
Copy link
Contributor

mwetter commented Sep 20, 2023

Ouch! I can reproduce it too and missed this in the review. I already integrated the updates in Buildings, and rerouted various connections to the new connector placements. At this point, I think it would actually be faster, and more importantly add more value to users, to make new additional base classes that have the connectors on the top and bottom, and use them for the tank. I can work on these changes and add them to PR #1795. As they are only needed by the tank, I plan to make them subclasses of the tank, and once the bugs are fixed in the tools, we can restore what we merged.

@AntoineGautier
Copy link
Contributor Author

@mwetter @FWuellhorst FYI, I just got this feedback from Dassault Systemes after submitting this issue to them.

It looks like the issue has already been solved for the coming Dymola 2024x. I invite you to test if the correction is working as soon as this new release of Dymola is available.

@AntoineGautier
Copy link
Contributor Author

Closed as completed with #1795

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

No branches or pull requests

3 participants