Opened 8 years ago

Last modified 3 years ago

#4372 accepted defect

model replaceable path changes when the model icon is moved and the model becomes no more replaceable

Reported by: Andrea.Bartolini Owned by: adrpo
Priority: blocker Milestone: 1.19.0
Component: OMEdit Version: v1.12.0
Keywords: Cc:

Description (last modified by casella)

Please consider the following models (also attached):

package P1
  model M1
    extends Modelica.Blocks.Interfaces.SISO;
    Boolean Flag = false;
    Real x = if Flag then u else 0;
  equation
    y = 0.5*x;
  end M1;

  model M2
    extends Modelica.Blocks.Interfaces.SISO;
    replaceable model Mx = P1.M1;
    Mx mx annotation(Placement(visible = true, transformation(origin = {-2, 0}, extent = {{-10, -10}, {10, 10}}, rotation = 0)));
  equation
  connect(u, mx.u) annotation(Line(points = {{-120, 0}, {-14, 0}}, color = {0, 0, 127}));
  connect(mx.y, y) annotation(Line(points = {{9, 0}, {110, 0}}, color = {0, 0, 127}));
  end M2;

  model M3
    P1.M2 m2(redeclare model Mx=P1.M1(Flag=true)) annotation(Placement(visible = true, transformation(origin = {-2, 0}, extent = {{-10, -10}, {10, 10}}, rotation = 0)));
    Modelica.Blocks.Sources.Step step1(startTime = 0.5)  annotation(Placement(visible = true, transformation(origin = {-36, 0}, extent = {{-10, -10}, {10, 10}}, rotation = 0)));
  equation
    connect(step1.y, m2.u) annotation(Line(points = {{-24, 0}, {-14, 0}}, color = {0, 0, 127}));
  end M3;
end P1;

The model M2 defines the model Mx as replaceable and the model M3 uses M2 redeclaring the model Mx in order to change the Flag parameter value from false to true.

Simulating the model M3 the results is according I expect, the Flag parameter value is changed (from false to true) and the step is propagated to the model m2 output.

Now open the model M2 and simply move the model mx

the line:

    Mx mx annotation(Placement(visible = true, transformation(origin = {-2, 0}, extent = {{-10, -10}, {10, 10}}, rotation = 0)));

is changed by OmEdit in:

P1.M2.Mx mx annotation(Placement(visible = true, transformation(origin = {-2, 0}, extent = {{-10, -10}, {10, 10}}, rotation = 0)));

and the model Mx now seems to be no more replaceable, in fact, re-simulating the model M3 the Flag parameter value now is not propagated and the output y of model m2 stays fixed to 0.

Attached are: package P1, output before M1 moving, output after M1 moving.

OMEdit 1.12.0~dev-250-g0f5c07d
Connected to OpenModelica 1.12.0~dev-561-gc489db1
sysop: Ubunti 14.04 - 34 bit

Attachments (3)

BeforeMoving.png (21.2 KB) - added by Andrea.Bartolini 8 years ago.
AfterMoving.png (21.0 KB) - added by Andrea.Bartolini 8 years ago.
P1.mo (1.1 KB) - added by Andrea.Bartolini 8 years ago.

Download all attachments as: .zip

Change History (25)

Changed 8 years ago by Andrea.Bartolini

Changed 8 years ago by Andrea.Bartolini

Changed 8 years ago by Andrea.Bartolini

comment:1 Changed 8 years ago by janK

This is an interesting ticket. I think it is related to #4351.

comment:2 Changed 8 years ago by Andrea.Bartolini

  • Summary changed from replaceable path changes when a model icon is moved to model replaceable path changes when a the model icon is moved and the model becomes no more replaceable

comment:3 Changed 8 years ago by Andrea.Bartolini

  • Summary changed from model replaceable path changes when a the model icon is moved and the model becomes no more replaceable to model replaceable path changes when the model icon is moved and the model becomes no more replaceable

comment:4 follow-up: Changed 8 years ago by adrpo

The problem is that we (wrongly) fully qualify the type of components when OMEdit asks for them, which means that any component update that OMEdit does will replace the type with the fully qualified one.
Fully qualifying component which have local types lead to missing modifications.
@janK: this has nothing to do with imports (except using imports to fully qualify the types).

comment:5 Changed 8 years ago by janK

@adrpo: Yes, I meant exactly that case when import is used to fully qualify the types. The wrong behavior (paths are replaced by OMEdit) was similar, this is why I referred to my ticket.

comment:6 Changed 8 years ago by adrpo

  • Owner changed from adeas31 to adrpo
  • Status changed from new to accepted

comment:7 in reply to: ↑ 4 ; follow-up: Changed 7 years ago by casella

Replying to adrpo:

The problem is that we (wrongly) fully qualify the type of components when OMEdit asks for them

Do we have a plan to avoid this?

comment:8 in reply to: ↑ 7 Changed 7 years ago by adrpo

Replying to casella:

Replying to adrpo:

The problem is that we (wrongly) fully qualify the type of components when OMEdit asks for them

Do we have a plan to avoid this?

Yes. The OMC API should not fully qualify these, especially if they are pointing to a local replaceable model. This happens also with the replaceable support I implemented for #2079 and I'm trying some solutions now for both these tickets.

comment:9 Changed 7 years ago by adeas31

  • Milestone changed from 1.12.0 to 1.13.0

comment:10 Changed 6 years ago by casella

  • Milestone changed from 1.13.0 to 1.14.0

Rescheduled to 1.14.0

comment:11 Changed 5 years ago by casella

@adrpo, will this issue be fixed in the new API when it's finally rolled out?

comment:12 Changed 5 years ago by casella

  • Milestone changed from 1.14.0 to 1.15.0

Releasing 1.14.0 which is stable and has many improvements w.r.t. 1.13.2.

This issue, previously marked as blocker for 1.14.0, is rescheduled to 1.15.0

comment:13 Changed 4 years ago by casella

  • Milestone changed from 1.15.0 to 1.16.0

Release 1.15.0 was scrapped, because replaceable support eventually turned out to be more easily implemented in 1.16.0. Hence, all 1.15.0 tickets are rescheduled to 1.16.0

comment:14 Changed 4 years ago by casella

  • Description modified (diff)

comment:15 Changed 4 years ago by casella

  • Milestone changed from 1.16.0 to 1.16.1

@adrpo has a fix in the pipeline to avoid this issue; the idea is to also store the actual relative path besides the fully qualified one, so that when the component declaration with the changed modifiers is re-added to the code, the original relative path can be put there instead.

This fixes the issue, and probably also improves the usability with revision control systems, since it reduces the chances of bogus changes to the source code.

Last edited 4 years ago by casella (previous) (diff)

comment:16 Changed 4 years ago by casella

  • Milestone changed from 1.16.1 to 1.16.0

comment:17 Changed 4 years ago by adrpo

  • Milestone changed from 1.16.0 to 1.16.1

comment:18 Changed 4 years ago by casella

@adrpo, should we move this to 1.17.0? We should release 1.16.1 which fixes regressions ASAP.

comment:19 Changed 4 years ago by adrpo

  • Milestone changed from 1.16.1 to 1.17.0

Let's move this to 1.17.0.

comment:20 Changed 4 years ago by casella

  • Milestone changed from 1.17.0 to 1.18.0

Retargeted to 1.18.0 after 1.17.0-dev.beta2 release

comment:21 Changed 3 years ago by casella

  • Milestone 1.18.0 deleted

Ticket retargeted after milestone closed

comment:22 Changed 3 years ago by casella

  • Milestone set to 1.19.0

1.18.0 blocker tickets moved to 1.19.0

Note: See TracTickets for help on using tickets.