NFTyping.typeComponents can't evaluate structural parameter (hard)

[Francesco Casella]

Please check ​PowerSystems_PowerSystems.Examples.Wind.WindTurbine_PSGR. The NF fails during NFTyping.typeComponents with this error

[OMCompiler/build/lib/omlibrary/PowerSystems 0.6.0/AC3ph/Machines.mo:2240:3-2240:48:writable]
Error: Could not evaluate structural parameter (or constant): 
generator.c.n_d which gives dimensions of array: L_rd. 
Array dimensions must be known at compile time.

I've seen this kind of error in many other places, also in other libraries.

[Francesco Casella]

See also #3186

[Per Östlund]

Minimal model

[Per Östlund]

I added a minimal model that produces the same error, both with the old and the new frontend.

Note to self:
It seems rather complicated to get this to work. The first issue encountered with the NF is that it tries to evaluate a ones call inside the polyTime function during typing of the function, which fails because it has an unknown size. We should probably not evaluate fill (ones is just handled as fill(1, ...)) during typing at all, but rather in e.g. SimplifyModel. Or not at all, but I think it's currently done because the backend needs it.

Turning evaluation of fill off lets the model get further, but then it fails in function evaluation instead. It can't evaluate n_d which is used in machineSyn, because we apply input replacements only after adding local and output replacements. And they are needed to evaluate the dimension of e.g. zx_d in that function. We probably need to sort the parameters and add them in order of dependency, and apply replacements as we go along instead of at the end.

And if that issue is worked around the model still fails due to other issues.

[Francesco Casella]

I added a remark in the title that this is a hard issue

[Francesco Casella]

@perost, we discussed this with Adrian during the Board meeting this morning. As I understand, the issue is made hard by the requirement of not expanding arrays.

This morning we made the decision to make the NF the default choice in 1.14.0, with an automatic fallback to the OF in OMEdit in case the code generation fails. It would be nice if we could get past this issue, so that the NF coverage is actually better than the OF when we make the release, tentatively end of June.

I wonder if we could take the same approach we followed to address the non-vectorized backend limitations. As a temporary measure, if -d=nfScalarize is false, then expand the arrays and proceed as with the OF. The current backend needs that anyway, so in the meantime we increase the coverage and we don't have any actual penalty on performance. Later on, you can take all the time you need to fix the non-expanded version, without being on the critical path for the release.

Support of non-expanding arrays is a very important development for the future, but releasing 1.14.0 and 2.0.0, which won't have all-out support for this feature, has a much higher strategic value for the OSMC right now, so it should have higher priority.

If my proposal is feasible, and unless you are almost through with this issue, I would then suggest that you postpone the development for non-expanded arrays for a while and focus on the NF remaining NF 2.0.0 open issues, so we can get as many as possible in for 1.14.0 and get them all fixed in time for the 2.0.0 release this fall.

What do you think?

[Per Östlund]

Replying to casella:

What do you think?

I think this might be the wrong ticket? I guess the issue you're really referring to is the array modifier issue, which we have many tickets about.

For that issue it's not really possible to just expand the arrays, the issue is an integral part of the NF. We already do expand array components in the later phases of the NF since that's required for the backend as you say, but one of the problems caused by that issue is that e.g. bindings become wrong which causes the scalarization to fail.

[Francesco Casella]

Replying to perost:

I think this might be the wrong ticket?

I thought this ticket also depended on that issue, but I was apparently wrong.

I guess the issue you're really referring to is the array modifier issue, which we have many tickets about.

Can you list them here? Besides #5202, it's not obvious to me which ones do you refer to.

For that issue it's not really possible to just expand the arrays, the issue is an integral part of the NF. We already do expand array components in the later phases of the NF since that's required for the backend as you say, but one of the problems caused by that issue is that e.g. bindings become wrong which causes the scalarization to fail.

I see. I'm just a bit worried that the fixing of all the other (hopefully easier to handle) NF issues gets delayed indefinitely. If this issue is going to take a long time to be addressed, could we re-assess the priority of the other remaining NF tickets?

[Per Östlund]

Replying to casella:

Replying to perost:

I think this might be the wrong ticket?

I thought this ticket also depended on that issue, but I was apparently wrong.

I guess the issue you're really referring to is the array modifier issue, which we have many tickets about.

Can you list them here? Besides #5202, it's not obvious to me which ones do you refer to.

#5464, #5423, #5373, #5323, #5241, #5202, #5466 are all probably the same issue. I might have missed some, I just quickly scanned the ticket list for the NF.

For that issue it's not really possible to just expand the arrays, the issue is an integral part of the NF. We already do expand array components in the later phases of the NF since that's required for the backend as you say, but one of the problems caused by that issue is that e.g. bindings become wrong which causes the scalarization to fail.

I see. I'm just a bit worried that the fixing of all the other (hopefully easier to handle) NF issues gets delayed indefinitely. If this issue is going to take a long time to be addressed, could we re-assess the priority of the other remaining NF tickets?

I've been working under the assumption that we'd switch to the NF by default in 2.0.0, so lately I've mostly been working on 1.14.0 issues rather than the array modifier issue (I've fixed #4297 for example, I just haven't had time to push it in yet due to the git changes and meetings). I usually fix easy tickets as soon as possible, so the remaining tickets are either hard or not very important.

[Francesco Casella]

Replying to perost:

#5464, #5423, #5373, #5323, #5241, #5202, #5466 are all probably the same issue. I might have missed some, I just quickly scanned the ticket list for the NF.

OK, thanks. I'll add a comment to them.

I've been working under the assumption that we'd switch to the NF by default in 2.0.0, so lately I've mostly been working on 1.14.0 issues rather than the array modifier issue (I've fixed #4297 for example, I just haven't had time to push it in yet due to the git changes and meetings). I usually fix easy tickets as soon as possible, so the remaining tickets are either hard or not very important.

OK.

Sorry for the change of plan, this came out today. Given the huge delay in delivering 1.14.0, I think it is a good decision. Given the analysis reported in #5490 and the latest proposal regarding handling of failures in OMEdit, I can't see any serious drawbacks, while I see a number of good reasons to do it. First and foremost, most users have no idea of the technical background, so they will never set the NF by themselves, because they have no idea what that implies; in fact, a lot of users keep using old versions like 1.12.0 or 1.11.0, God forbid. In case they are annoyed by the NF, they can just click on a button and get rid of it. But my guess is that most people will rather be happy because of increased speed and coverage.

The only way to move forward is to make the NF active by default now. Releasing 2.0.0 will also require fixing a number of other issues that do not depend on the NF (e.g. respecting the Windows installation standards and supporting directories with UTF-8 characters or long names, which will be another nightmare for Adrian), which may futher delay it to the end of this year. I think the NF is mature enough now and we shouldn't way this long for the bulk of the users to at least try it out.

BTW, at Dynamica we are almost exclusively using OMC with the NF; in fact, many recent developments would not have been possible at all with the OF.

[Per Östlund]

Fixed since c1c920a9.

[Francesco Casella]

Very good! I'm looking forward to seeing the results of the Jenkins test runs (also for the other commits of today).

[Per Östlund]

Replying to casella:

Very good! I'm looking forward to seeing the results of the Jenkins test runs (also for the other commits of today).

Expect significant improvements for PowerSystems, but not so much for Buildings. I though Buildings had much the same issues as PowerSystems since they produced the same error messages, but it seems Buildings has its own unique issues.

[Per Östlund]

The e-mail reporting is currently a bit broken (Adrian is trying to fix that the e-mails are sent before the results are available), but the ​results from the first commits are now in. So far so good, the regressions are just unstable models and not due to my changes.

[Francesco Casella]

This issue is now resolved, the model is still broken but for other reasons.

[Per Östlund]

All models in ​PowerSystems now pass the frontend at least, except for ​PowerSystems.Examples.Wind.WindTurbine_PSGI which is missing an each. Two other models crash due to #5822 and is therefore missing statistics, but that's a backend issue.