Opened 5 years ago

Last modified 5 years ago

#5688 new defect

Use of removed conditional component not reported

Reported by: Mahder Alemseged Gebremedhin Owned by: Per Östlund
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 by Per Östlund, 5 years ago

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 by Mahder Alemseged Gebremedhin, 5 years ago

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 5 years ago by Mahder Alemseged Gebremedhin (previous) (diff)

comment:3 by Francesco Casella, 5 years ago

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 by Mahder Alemseged Gebremedhin, 5 years ago

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.

in reply to:  3 ; comment:5 by Per Östlund, 5 years ago

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.

in reply to:  3 comment:6 by Mahder Alemseged Gebremedhin, 5 years ago

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.

Version 0, edited 5 years ago by Mahder Alemseged Gebremedhin (next)

in reply to:  5 comment:7 by Francesco Casella, 5 years ago

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.