Opened 4 years ago

Last modified 4 years ago

#5688 new defect

Use of removed conditional component not reported

Reported by: mahge930 Owned by: perost
Priority: normal Milestone: Future
Component: New Instantiation Version:
Keywords: Cc:

Description

class A
  parameter Boolean a = false;
  parameter Real b if a;
  Real v;
equation
  v = time * b;
end A;

Does not report an error for use of b in the equation.

This causes problems on the generated code.

E.g.
https://libraries.openmodelica.org/branches/newInst/ThermoPower/files/ThermoPower_ThermoPower.PowerPlants.SteamTurbineGroup.Tests.TestTurbineHPefficiency.err

Change History (7)

comment:1 Changed 4 years ago by perost

You don't get an error but a warning:

Warning: Conditional component b is used in a non-connect context.

Making it an error would of course be preferable, but I think that caused too many library issues (even the MSL had issues like this, but they might've been fixed now). So we need to make sure the libraries are fixed before we can completely forbid this.

comment:2 Changed 4 years ago by mahge930

I see. I think the models should/would not compile (generated code) anyway since the variable is not declared. Instead of failing at generated code compilation they should fail somewhere earlier if possible.

It is also a fault of simulation codegen because it does not actually report failure of lookup for these sim variables. It just generates them from the cref. I will check more.

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

comment:3 follow-ups: Changed 4 years ago by casella

Regarding the ThermoPower model, I'm fine with fixing it, if that is necessary for better code generation.

I am still convinced that from a user's perspective this restriction is too strong, and that one should be allowed to use conditionally defined variables if they only show up in branches that are activated with the same parameters that activate the variables, but this is not the right place to discuss this issue, that belongs to the Modelica specification.

comment:4 Changed 4 years ago by mahge930

I see what you mean. What I am trying to do is minimize all the false negatives that I get at codegen, i.e., issues that are actually trickling down from FrontEnd and BackEnd. At least filter them out so they don't hide issues that are actually codegen related. Things like this and even worse cases where the compiler reports an error and then continues to codegen regardless.

Anyway these issues are not critical or blocker. I am creating them just to keep track of things. You keep doing your thing.

comment:5 in reply to: ↑ 3 ; follow-up: Changed 4 years ago by perost

Replying to casella:

I am still convinced that from a user's perspective this restriction is too strong, and that one should be allowed to use conditionally defined variables if they only show up in branches that are activated with the same parameters that activate the variables, but this is not the right place to discuss this issue, that belongs to the Modelica specification.

For the mentioned TestTurbineHPefficiency model there are no branches involved though, it's just straight up using a deleted component in an equation. If it had been used inside a branch activated by the same parameter it would probably have been fine.

comment:6 in reply to: ↑ 3 Changed 4 years ago by mahge930

Replying to casella:

Regarding the ThermoPower model, I'm fine with fixing it, if that is necessary for better code generation.

I am still convinced that from a user's perspective this restriction is too strong, and that one should be allowed to use conditionally defined variables if they only show up in branches that are activated with the same parameters that activate the variables, but this is not the right place to discuss this issue, that belongs to the Modelica specification.

Oh you are Francesco, I assumed I was talking to Per :)

Anyway like Per said above the model has to fail anyway. The component does not exist. It is just a question of where it should fail.

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

comment:7 in reply to: ↑ 5 Changed 4 years ago by casella

Replying to perost:

For the mentioned TestTurbineHPefficiency model there are no branches involved though, it's just straight up using a deleted component in an equation. If it had been used inside a branch activated by the same parameter it would probably have been fine.

The ThermoPower.PowerPlants package is in a state of poor maintenance, because we updated some interfaces some years ago in a non-backwards compatible way, and the corresponding update of that package has not been completed yet. I have a PhD guest student coming next week, we may take this opportunity to fix all these issues for good.

Note: See TracTickets for help on using tickets.