ModelicaSpecification icon indicating copy to clipboard operation
ModelicaSpecification copied to clipboard

Application of "each" when enclosing array is not the immediate parent

Open bilderbuchi opened this issue 4 years ago • 5 comments

Description

I have a Modelica model where I am initialising an array of submodels, and with OpenModelica I'm getting "Translation Warning 'each' used when modifying non-array element p" when setting fixed=true for Sub.p for all Sub elements in an array. See the below MWE.

Steps to Reproduce

model test_each_fixed
  model Sub
    Real p;
  end Sub;
  // gives Translation Error Non-array modification ‘true‘ for array component 
  // ‘fixed‘, possibly due to missing ‘each‘.
  //Sub sub1[6](p(start=linspace(0.0, 1.0, 6), fixed=true)); //Variant A
  
  // fair enough, let's add an "each":
  
  // gives Translation Warning 'each' used when modifying non-array element p.
  Sub sub2[6](p(start=linspace(0.0, 1.0, 6), each fixed=true)); //Variant B
  
  // works without warning, but having to use an explicit fill instead of each seems wrong
  //Sub sub3[6](p(start=linspace(0.0, 1.0, 6), fixed=fill(true,6))); //Variant C
  
  // also generates the warning (probably could be expected)
  //Sub sub4[6](p.start=linspace(0.0, 1.0, 6), each p.fixed=true); //Variant D
equation
  // Done in a clunky way to avoid distractions from more "each" usage.
  der(sub2[1].p) = 1;
  der(sub2[2].p) = 1;
  der(sub2[3].p) = 1;
  der(sub2[4].p) = 1;
  der(sub2[5].p) = 1;
  der(sub2[6].p) = 1;
end test_each_fixed;

Expected Behavior

I thought that Variant B should work without emitting a warning (@HansOlsson agrees). Effectively this is the same as Variant C, but having to explicitly fill with an array of matching shape feels like it should not be necessary.

Request for clarification

The key point is that (as per @perost, emphasis mine)

The specification is ambiguous on whether each applies to the immediately enclosing array or if it can be used in a nested modifier as long as one of the modified elements is an array.

In OM we interpret it such that it only applies to the immediately enclosing array, i.e. in a(b(each ...)) we say that b should be an array, but it seems like Hans interprets it to mean that it's also fine if a is an array when b is a scalar. I guess we could change OM to use the looser definition, we only give a warning right now anyway, but it would be good get some clarification.

In my understanding, the salient point is how in this sentence

The each keyword on a modifier requires that it is applied in an array declaration/modification

the "in" is interpreted. Arguably in sub2, each is applied in (i.e., within) an array declaration/modification, just not directly as p() is not an array declaration/modification.

Relevant context/previous discussions

This has been discussed in https://github.com/OpenModelica/OpenModelica/issues/7777 already, without a clear conclusion. @casella asked me to ask for clarification here.

I had already asked this on StackOverflow, and @HansOlsson has indicated there that Variant B should work without a warning.

Relevant standard section 7.5.2 https://specification.modelica.org/master/inheritance-modification-and-redeclaration.html#modifiers-for-array-elements

Probably related: #2630

Version and OS

This is using OpenModelica v1.18.0 (64-bit) on Windows 10 64 bit

bilderbuchi avatar Nov 30 '21 09:11 bilderbuchi

Actually there are three possible variants of each, not only the first two as suggested by the question, and it is the third one that is used:

  1. each applies to the immediately enclosing array
  2. each applies to the nearest enclosing array
  3. each applies to all enclosing arrays

The text supporting that is "In case of nested modifiers this implies it is applied individually to each element of each element of the enclosing array; see example" and the corresponding example: B b2 [2]( c ( each a = {1 , 2 , 3} , ...) ;

Can you suggest some improvement that makes that clearer?

HansOlsson avatar Dec 07 '21 11:12 HansOlsson

@perost may want to comment

casella avatar Dec 07 '21 15:12 casella

The text supporting that is "In case of nested modifiers this implies it is applied individually to each element of each element of the enclosing array; see example" and the corresponding example: B b2 [2]( c ( each a = {1 , 2 , 3} , ...) ;

I first misread that sentence and assumed "each element of each element of" was a typo, but I guess that makes sense if you think about it a bit. Maybe it could be improved to make it clearer, but I don't know how.

But in that case both b2 and c are arrays, which is fine to me. But what if c is not an array? I.e. something like:

model B
  Real c;
end B;

model M
  B b[2](c(each start = 1.0));
end M;

In that case OpenModelica gives a warning, because we only check if the closest enclosing name refers to an array.

perost avatar Dec 07 '21 16:12 perost

Can you suggest some improvement that makes that clearer?

In this passage:

The each keyword on a modifier requires that it is applied in an array declaration/modification, and...

Note that in Per's example (which is equivalent to mine, but simpler), c(each start = 1.0), each is not applied in an array modification, but nested inside (an enclosing) one.

Some variations that would make my understanding clearer that the array does not have to be the immediate enclosing name (to be refined into "proper" standardese):

  • "The each keyword on a modifier requires that it is applied inside of an array declaration/modification"
  • "The each keyword on a modifier requires that it must be contained in an array declaration/modification"

It would also help to add an example that demonstrates the salient points of the presently discussed case, maybe this:

model G
  C c[2] (each a = {1, 2, 3}, d(start={4, 5}, each fixed=true));
end G;

By the way, there is a typo in this definition (in master):

 model B
  C c[5](each a = {1, 2, 3}, d = {1, 2, 3, 4, 5});
  parameter Real b = 0; // has to be named "p" to be consistent with the other snippets
end B; 

bilderbuchi avatar Dec 09 '21 08:12 bilderbuchi

I'm not sure what is the best way to formulate the specification, but from and end-user perspective, I would find it very odd that

Sub sub2[6](p(start=linspace(0.0, 1.0, 6), each fixed=true));

is not accepted, and one needs to write

Sub sub2[6](p(start=linspace(0.0, 1.0, 6), fixed = fill(true, 6)));

So, I also agree with @HansOlsson that the first variant is perfectly fine and should be accepted without warnings.

casella avatar Jan 13 '22 13:01 casella