Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implicit iterator range and expandable arrays. #2560

Closed
qlambert-pro opened this issue May 6, 2020 · 4 comments · Fixed by #2589
Closed

Implicit iterator range and expandable arrays. #2560

qlambert-pro opened this issue May 6, 2020 · 4 comments · Fixed by #2589
Labels
decided A decision has been made (label added before the spec is changed) discussion Indicates that there's a discussion; not clear if bug, enhancement, or working as intended
Milestone

Comments

@qlambert-pro
Copy link
Collaborator

I was wondering if the following model is valid:

model AugmentArrayLoop
  expandable connector EC
  end EC;

  connector RealOutput = output Real;

  model M
    RealOutput ro = 1.0;
  end M;

  EC ec;
  M m;

  Real x[Boolean];
equation
  for i loop
    x[i] = 0;
    connect(m.ro, ec.ro[i]);
  end for;
end AugmentArrayLoop;

I couldn't find anything forbidding it, so I suppose it is.

@HansOlsson
Copy link
Collaborator

I agree that there is currently nothing forbidding it, but I believe it should be:

Specifically having an implicit iterator range based on an array in an expandable connector.
The reason is that the members of the expandable connector are also implicitly defined and combining two implicit definitions is not a good idea.

Note that implicit iterator ranges require that all indexed arrays have the same size, i.e. we cannot say that x determines the size and ec.ro just tags along.

The Boolean indexing of ec.ro seems odd, but I don't see any particular reason to forbid it.

@HansOlsson HansOlsson added the discussion Indicates that there's a discussion; not clear if bug, enhancement, or working as intended label Jun 10, 2020
@HansOlsson HansOlsson added this to the Phone2020-3 milestone Jun 10, 2020
@qlambert-pro
Copy link
Collaborator Author

qlambert-pro commented Jun 10, 2020

Note that implicit iterator ranges require that all indexed arrays have the same size, i.e. we cannot say that x determines the size and ec.ro just tags along.

This is exactly what we do, and it works. we just disallow loops where the only usage of the iterator is as a subscript of a member of an expandable connector.

@HansOlsson
Copy link
Collaborator

Note that implicit iterator ranges require that all indexed arrays have the same size, i.e. we cannot say that x determines the size and ec.ro just tags along.

This is exactly what we do, and it works. we just disallow loops where the only usage of the iterator is as a subscript of a member of an expandable connector.

Ok, that would be possible but it feels odd to me, and that's not part of the spec:
https://specification.modelica.org/master/statements-and-algorithm-sections.html#implicit-iteration-ranges

If it is used to subscript several expressions, their ranges must be identical. The IDENT may also, inside a reduction-expression, array constructor expression, for-statement, or for-equation, occur freely outside of subscript positions, but only as a reference to the variable IDENT, and not for deducing ranges.

@HansOlsson
Copy link
Collaborator

HansOlsson commented Jun 10, 2020

Rewriting:

   for i in 1:size(y,1) loop
     y[i] = 0;
     connect(m.ro, ec.ro[i]);
  end for;

or:

   for i /* in 1:size(y,1) */ loop
     y[i] = 0;
   end for;
   for i in 1:size(y,1) loop
     connect(m.ro, ec.ro[i]);
  end for;

I thus see two possibilities:

  • State that IDENT for implicit iteration ranges may not be used in subscript for arrays in expandable connector components. Martin, Filippo
  • State that IDENT for implicit iteration ranges may be used in subscripts for arrays in expandable connector components, but only as a reference to the variable IDENT and not for deducing ranges. Quentin, Henrik, Adrian, Markus, Axel, Stefan

Go for 2nd option.

Gerd, Christoff, Dag, Hans abstain
Separately: And forbid/restrict size of array in expandable connector?

@HansOlsson HansOlsson added the decided A decision has been made (label added before the spec is changed) label Jun 11, 2020
HansOlsson added a commit to HansOlsson/ModelicaSpecification that referenced this issue Jun 18, 2020
henrikt-ma added a commit to henrikt-ma/ModelicaSpecification that referenced this issue Jun 22, 2020
Implements the part of the decision in modelica#2560 that wasn't included in modelica#2589.
HansOlsson added a commit that referenced this issue Jun 22, 2020
No deduction of iterator range from array in bus
Closes #2560
HansOlsson pushed a commit that referenced this issue Oct 8, 2020
Implements the part of the decision in #2560 that wasn't included in #2589.
* Restrict use of 'size' on components of expandable connectors
* Tidy up function calls that are parameter expressions; 
* Improve readability of source of nested item list on expandable connectors; Stumbled upon these when looking for specification of 'size'.
* Also throwing in some missing \lstinline.
* Say 'Error', not 'Not allowed' in listing comments
* Restrict uses of sizeless array components of expandable connectors
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
decided A decision has been made (label added before the spec is changed) discussion Indicates that there's a discussion; not clear if bug, enhancement, or working as intended
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants