Local balance per type#3818
Conversation
HansOlsson
left a comment
There was a problem hiding this comment.
I assume it was intended to be "Ready for Review", after merging the other one, right?
Looks good overall.
However, there are some minor comments.
Co-authored-by: Hans Olsson <HansOlsson@users.noreply.github.com>
|
Language group: Will be a clear separation integer/enumeration; so cannot write an equation with inverse for that (example to be given). |
|
Here is an example where separating integers/enumerations would break a currently working model: model IntEnumExample
type E = enumeration(A, B);
function intToE
input Integer i;
output E e /*= E(i)*/;
protected
E values[:] = {E.A, E.B};
algorithm
e := values[i];
annotation(Inline=false, inverse(i = eToInt(e)));
end intToE;
function eToInt
input E e;
output Integer i = Integer(e);
algorithm
annotation(Inline=false, inverse(e = intToE(i)));
end eToInt;
input Integer i(min=1, max=2, start=1);
E e;
equation
i = eToInt(e);
end IntEnumExample;This model currently compiles and simulates in Modelon Impact. But it would not be valid with the proposed type separation, the only unknown variable has type |
SystemModeler does not accept the model based on the following rule in https://specification.modelica.org/master/modelica-dae-representation.html:
|
|
Dymola doesn't accept it either. |
You are right. In that case I have no concerns, should be fine to separate integers/enumerations. |
Good enough as start
HansOlsson
left a comment
There was a problem hiding this comment.
Ok now. Should preferably clean up the type compatible expression part.
Fixing #3763. Opening as draft to make clear that this is not ready for merging, and so that a discussion about how to do this can be started already.
In particular, I am looking for input on which type concepts to use when defining scalar type class, that is the formal categorization used to break down the local balance counting based on type.
Note that this PR contains some language fixing commits which have also been isolated in the PR #3817, so the changes of the present PR will look much cleaner once #3817 has been merged.