Obscure error message from NF

[ceraolo]

Consider the following code:

model aaa
  parameter Real specificCons[:, :]( unit = "g/(kW.h)") = [0.0, 100; 10, 630];
equation
end aaa;

When checked with the OF I get the following very clear warning:

[1] 17:42:45 Translation Warning
[aaa: 2:38-2:55]: Non-array modification '"g/(kW.h)"' for array component, possibly due to missing 'each'.

When I check with the NF I get instead:

[2] 17:45:38 Translation Error
[aaa: 2:3-2:79]: Type mismatch in binding unit = "g/(kW.h)", expected subtype of String[2, 2], got type String.

which, IMO, is worse.

What makes the second message worse, for me, is:

• the phrase "subtype of" (with basic types such as Real, Integer, Boolean, String are involved can this be omitted?)
• the absence of the hint regarding each.

[casella]

@ceraolo, 'Subtype of XXX' is the technically correct description of derived types such as, e.g., SI.Units. Consider, for example

type Velocity = Real(unit = "m/s");

which is a subtype of Real.I don't see a problem with that.

However, I completly agree that if we can keep the hint about the missing 'each', when a scalar binding is found instead of an array one, this would be very helpful to users.

@perost, can you take care of that?

[ceraolo]

Replying to casella:

@ceraolo, 'Subtype of XXX' is the technically correct description of derived types such as, e.g., SI.Units. Consider, for example

type Velocity = Real(unit = "m/s");

which is a subtype of Real.I don't see a problem with that.

I think here we have a different situation.
It is just that String is a subtype of String. Removing "Subtype of", IMO clarifies.
I mean, the message would remain technically correct, but clearer.

[casella]

Well, any type is also a subtype of itself, the subtype relationship is not strict.

But I see your point. @perost, could we have a specific error message to display when the only thing that doesn't match in the bound type is the number of dimensions and their size? Could be something like:

Size mismatch in binding XXX. Expected array [2,2], got scalar.

In case the actual binding is a scalar and the expected one is any kind of array, you could add

Probably due to missing 'each'.

[perost]

I will change the error messages as per your suggestions, and currently I have it so that e.g.:

model M
  Real x[3] = {1, 2};
end M;

will result in the error:

Type mismatch in binding ‘x = {1, 2}‘, expected array dimensions [3], got [2].

While e.g.:

model M
  Real x[3] = 1;
end M;

will result in the error:

Non-array modification ‘1‘ for array component ‘x‘, possibly due to missing ‘each‘.

However, this:

model M
  Real x = {1, 2, 3};
end M;

will result in:

Type mismatch in binding ‘x = {1, 2, 3}‘, expected array dimensions [], got [3].

since that error is given when the base types are compatible but the dimensions are wrong. Personally I think using [] to denote a scalar is fine, but maybe you have a different opinion? I could trivially change it to give the old "expected subtype of" error when the component is a scalar instead, but I'd rather not add yet another error message just for this case. Besides, this particular case is quite rare in my experience.

[ceraolo]

Replying to casella:

Well, any type is also a subtype of itself, the subtype relationship is not strict.

That's what exactly I meant with the first dot of my ticket description and in comment 2!
So we finally agree on what we mean :-)

[perost]

Fixed in 31bdbe9.

[ceraolo]

since that error is given when the base types are compatible but the dimensions are wrong. Personally I think using [] to denote a scalar is fine, but maybe you have a different opinion? I could trivially change it to give the old "expected subtype of" error when the component is a scalar instead, but I'd rather not add yet another error message just for this case. Besides, this particular case is quite rare in my experience.

In think everything is much better now. As you noted, your latest example is slightly less self-explaining than the others. In case it is easy to change, I would prefer something like:
"expected array dimensions, got none"

Thank you.

[perost]

Replying to ceraolo:

since that error is given when the base types are compatible but the dimensions are wrong. Personally I think using [] to denote a scalar is fine, but maybe you have a different opinion? I could trivially change it to give the old "expected subtype of" error when the component is a scalar instead, but I'd rather not add yet another error message just for this case. Besides, this particular case is quite rare in my experience.

In think everything is much better now. As you noted, your latest example is slightly less self-explaining than the others. In case it is easy to change, I would prefer something like:
"expected array dimensions, got none"

Thank you.

It's actually the opposite, i.e. no array dimensions were expected in the binding. A better message might be something like "expected scalar, got array", but that would either make the error message harder to translate or I'd have to add a new message for this particular case.

But I think it's clear enough that most users will understand what it means, so unless someone objects heavily I think I'll keep it as it is now. I also think that at some point it becomes more of a hinderance than a help to have unique error messages for special cases, since it reduces the chance that the user is already familiar with the error and knows what it means.

[ceraolo]

It's actually the opposite, i.e. no array dimensions were expected in the binding.

I see! I did not take into account that we a re going from left to right.

But I think it's clear enough that most users will understand what it means, so unless someone objects heavily I think I'll keep it as it is now.

Ok. Thanks again for considering this ticket.