OpenModelica / OpenModelica

OpenModelica is an open-source Modelica-based modeling and simulation environment intended for industrial and academic usage.
https://openmodelica.org
Other
817 stars 305 forks source link

Potential bugs with exponentiation and implicit index #11275

Open mpradovelasco opened 11 months ago

mpradovelasco commented 11 months ago

Hi

This issue develops the comment of https://github.com/OpenModelica/OpenModelica/discussions/11274

Consider the following MWE:

package ExponentiationTest
  function F
    input Real c[:] "Real vectorial vector of coefficientes";
    input Real x[:] "Real vectorial input of transformation";
    output Real y[:] "Real vectorial output of transformation";
  algorithm
    y := {x[i].^c[i] for i};
  end F;
  function H
    input Real c[:] "Real vectorial vector of coefficientes";
    input Real x[:] "Real vectorial input of transformation";
    output Real y[:] "Real vectorial output of transformation";
  algorithm
    y := x.^c;
  end H;
  block M
    parameter Integer n = 3;
    parameter Real c[:]=fill(1,n);
    Real x[n] = {1,2,3};
    Real y[n];
  algorithm
    //y := F(c,x);
    y:= H(c,x);
  end M;
end ExponentiationTest;

When using the assignment y:=F(c,x) (commenting the other assignment) in block M, then OpenModelica gives following error when trying to check M:

[1] 12:30:43 Translation Error[CybSim.test: 1208:7-1208:30]: [Dimension 1 of c and 1 of x differs when trying to deduce implicit iteration range.](omeditmessagesbrowser:///CybSim.test?lineNumber=1208)

The error does not appear when the dimension is explicitly declared in formal parameters.

When using the assignment y:=H(c,x) (commenting the other) in block M, then OpenModelica gives a compilation error when trying to simulate (the model instantiate properly):

M_functions.c:16:24: error: use of undeclared identifier 'daeExpBinary'
real_array_copy_data(daeExpBinary:ERR for POW_ARR2, _y);
^
1 error generated.

The full text of the error is attached: ![[CProgram FilesOpenModelica1.23.0-de.txt]]

I think that these are two different bugs. One related to implicit ranges and the other with the compiler.

System: OMEdit 1.23-dev (23 Sept) with on Windows 11. Thanks

perost commented 11 months ago

For the issue with implicit ranges I'd say it's intended behaviour, because the requirement is that the indexed dimensions must be identical (the specification does not elaborate on what that means though). The compiler can't say whether the dimensions of c and x are identical since they depend on the input arguments. If the intention is that they always should be the same size then you should tell the compiler so by e.g. declaring x to be the same size as c:

  input Real c[:];
  input Real x[size(c, 1)];

This doesn't actually work right now though, because we didn't think of that case. But I made #11276 that improves the implicit range deduction to also take this case into account, so it should work as soon as that PR has been merged.

As for the other issue I guess @maghe is on it. The issue is that we don't actually have code generation for some of the exponentiation cases. This is complicated by the fact that the code we generate for the simple scalar ^ scalar case is quite involved and just generated directly by the code generation, so implementing the array cases with similar functionality might require some refactoring of the code.

mpradovelasco commented 11 months ago

Thank you for your prompt response. Certainly, although minor details may emerge, the advances in OpenModelica are really very very interesting. I congratulate all you for your huge work.

mahge commented 11 months ago

@perost I know I might be giving you more work than you need but maybe we can handle this similar to how we handled slices in the NF. You implemented this in https://github.com/OpenModelica/OpenModelica/pull/8339.

Basically we try and convert them to array call statements. That is, change

y := x.^c;

to something like:

y := {x[$i]^c[$i] for $i in 1:size(x, 1)};

The slices handling has worked quite well so far. If it is not a lot of work (hopefully you can reuse some parts of https://github.com/OpenModelica/OpenModelica/pull/8339). Doing it in Codegen is probably going to be so complicated. Like you said just handling the scalar case is more complicated than one would expect. We can probably loop over the values but we will essentially be doing the same thing as the array call constructors but in a much more ugly and complicated way.

P.S. Sorry about the previous half finished comment if you got the notifications. I think I pressed CTRL+ENTER and that might be a GitHub shortcut.