Skip to content

Conversation

@Callejn
Copy link

@Callejn Callejn commented Mar 6, 2017

The inputs/outputs of Modelica.Blocks.Interfaces.{DiscreteSISO, DiscreteMIMO, DiscreteMIMOs} should be marked as discrete.

The inputs/outputs of Modelica.Blocks.Interfaces.{DiscreteSISO, DiscreteMIMO, DiscreteMIMOs} should be marked as discrete.
@beutlich beutlich added the L: Blocks Issue addresses Modelica.Blocks label Mar 6, 2017
@beutlich beutlich added this to the MSL_next-MINOR-version milestone Mar 6, 2017
@HansOlsson
Copy link
Contributor

HansOlsson commented Mar 6, 2017

No. They should not be marked as discrete - and this pull request shall not be merged.

The discrete keyword in Modelica indicate that the variable is assigned in a when-clause; but a variable can be discrete in a broader sense without this keyword.

The inputs are clearly not given by a when-statement (since their values are given by a connect-statement).

In some cases it is right for the outputs - but it depends, e.g. it is wrong for: Modelica.Blocks.Discrete.ZeroOrderHold - and likely also for user models.

See also #2123

@beutlich beutlich requested a review from HansOlsson March 6, 2017 12:32
Copy link
Contributor

@HansOlsson HansOlsson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reject addition of 'discrete'. The issue is that discrete in Modelica as a keyword mean assigned in a when-clause, and is not even needed in that case. For the inputs it is never correct (they are connected to). For the outputs it is sometimes incorrect (and often correct); and thus this would break some existing models (including one in MSL). The reason for 'discrete' just meaning 'assigned in when-clause' is that it is straightforward to describe and test - whereas a more general 'discrete' would require specifying a decomposition of models etc. From a user-perspective "discrete" is not that intuitive to use, if we don't know when it changes, that is improved with the synchronous addition in Modelica 3.3.

@HansOlsson
Copy link
Contributor

However, the fact that we have gotten two reports about this issue indicate that we might add the explanation from #2123 to the specific components.

@dietmarw
Copy link
Member

dietmarw commented Oct 1, 2017

This PR is superseded by #2271

@dietmarw dietmarw closed this Oct 1, 2017
@dietmarw dietmarw modified the milestones: MSL_next-MINOR-version, never Oct 1, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

L: Blocks Issue addresses Modelica.Blocks

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants