OpenModelica icon indicating copy to clipboard operation
OpenModelica copied to clipboard

Elementwise exponentiation in functions is broken

Open bilderbuchi opened this issue 1 year ago • 9 comments

We are trying to implement a function doing (among other things) a simple elementwise exponentiation over two arrays. AFAICT, this is OK according to https://specification.modelica.org/master/arrays.html#element-wise-exponentiation

model exponentiation

Real[2] x={1,1};
Real[2] y={1,1};
Real[2] z;

function expo
  input Integer size;
  input Real[size] x;
  input Real[size] y;
  output Real[size] z;
algorithm
  z := x .^ y;
end expo;

equation
z = expo(2,x,y);
end exponentiation;

Simulating this model fails compilation with

exponentiation_functions.c:16:24: error: use of undeclared identifier 'daeExpBinary' 16 | real_array_copy_data(daeExpBinary:ERR for POW_ARR2, _z);

a) This error message is confusing/not very understandable for end users b) AFAICT this should not error in the first place, and should yield a result of z={1,1} .

A potential second (related?) issue is that if one implements this in a more generic way with flexible array sizes:

model exponentiation_silenterror

Real[2] x={1,1};
Real[2] y={1,1};
Real[2] z;

function expo
  //input Integer size;
  input Real[:] x;
  input Real[:] y;
  output Real[:] z;
algorithm
  z := x .^ y;
end expo;

equation
z = expo(x,y);
end exponentiation_silenterror;

AFAICT this is also according to spec (all array sizes are known when the function is run), but here I am less sure. When simulating, OMEdit just very quickly flashes "Translating exponentiation_silenterror" in the OMEdit status bar, and nothing else happens. I could not find an indication I could understand in the log as to what goes wrong here. 🤷

OpenModelica v1.25.0-dev-174-g92fc808a19, windows 10

bilderbuchi avatar Nov 27 '24 14:11 bilderbuchi

exponentiation_functions.c:16:24: error: use of undeclared identifier 'daeExpBinary' 16 | real_array_copy_data(daeExpBinary:ERR for POW_ARR2, _z);

a) This error message is confusing/not very understandable for end users

C compilation errors should never take place. If they do, it's because there's something broken in the compiler that generates broken C code. Of course this needs to be debugged and fixed by the developers, and of course it is not really possible to emit better error messages, since these things are unforseen and should ideally not take place.

@bilderbuchi should we make this clearer in the User's Guide? If so, where?

casella avatar Nov 27 '24 15:11 casella

Here is a comprehensive collection of MWEs:

package ElementWiseExponentiation
  model MWE1
    function f
      input Real x[2];
      output Real y[2];
    algorithm
      y := x.^x;
    end f;
  
    Real x[2] = f({time, 2*time});  
  end MWE1;
  
  model MWE2
    function f
      input Integer size;
      input Real x[size];
      output Real y[size];
    algorithm
      y := x.^x;
    end f;
  
    Real x[2] = f(2, {time, 2*time});  
  end MWE2;
  
  model MWE3
    function f
      input Real x[:];
      output Real y[:];
    algorithm
      y := x.^x;
    end f;
  
    Real x[2] = f({time, 2*time});  
  end MWE3;
end ElementWiseExponentiation;

@phannebohm, @kabdelhak with the OB the first MWE works, but the other generate invalid C code. With the NB, non of the three works, for the same reason.

I guess this should be fixed ASAP.

casella avatar Nov 27 '24 15:11 casella

exponentiation_functions.c:16:24: error: use of undeclared identifier 'daeExpBinary' 16 | real_array_copy_data(daeExpBinary:ERR for POW_ARR2, _z);

a) This error message is confusing/not very understandable for end users

C compilation errors should never take place. If they do, it's because there's something broken in the compiler that generates broken C code. Of course this needs to be debugged and fixed by the developers, and of course it is not really possible to emit better error messages, since these things are unforseen and should ideally not take place.

@bilderbuchi should we make this clearer in the User's Guide? If so, where?

OK, I understand. No, basically it's fine -- I already suspected that that's what's going on (i.e. a problem related to generated code), but less experienced users are just stumped and very confused by such a message. Elsewhere in OM I think I've seen a prompt message like "this should not have happened, please report a bug with the developers". Maybe it would be good to add such a pointer if this kind of messages are emitted, if that is feasible with justifiable effort?

I guess this should be fixed ASAP.

Yeah, I agree about that :P

bilderbuchi avatar Nov 28 '24 07:11 bilderbuchi

These cases of exponentiation operator were apparently never implemented. So I guess we need to sit down, copy-paste some code and think about what it needs to do in order to compute x.^y correctly and in a memory-safe way. Guess what I'll do next week. :slightly_smiling_face:

You can implement the more generic way like this, which makes sure the sizes are compatible:

function expo
  input Real[:] x;
  input Real[size(x, 1)] y;
  output Real[size(x, 1)] z;
algorithm
  z := x .^ y;
end expo;

This lands back on the original issue, so once that is fixed this function should work. BTW, it already works if you only replace input Real[:] x; with input Real[2] x;.

phannebohm avatar Nov 28 '24 16:11 phannebohm

Elsewhere in OM I think I've seen a prompt message like "this should not have happened, please report a bug with the developers". Maybe it would be good to add such a pointer if this kind of messages are emitted, if that is feasible with justifiable effort?

Looks like a good idea. See #13310.

casella avatar Nov 28 '24 16:11 casella

Guess what I'll do next week. 🙂

Did you get anywhere? 🙂 Anything I can do to help?

bilderbuchi avatar Dec 11 '24 10:12 bilderbuchi

I didn't have time to implement it. Ideally we can isolate the code in https://github.com/OpenModelica/OpenModelica/blob/3a2e2cb29cb52067783a76f1a0fe93e006962e25/OMCompiler/Compiler/Template/CodegenCFunctions.tpl#L5746-L5843 into its own function (perhaps in the C runtime) and call it in all places in https://github.com/OpenModelica/OpenModelica/blob/3a2e2cb29cb52067783a76f1a0fe93e006962e25/OMCompiler/Compiler/Template/CodegenCFunctions.tpl#L5923-L5926

@bilderbuchi, if you are familiar with Susan and our C runtime you can take care of that. Please notify me if you do. :slightly_smiling_face: Otherwise I will do it as soon as I get to it.

phannebohm avatar Dec 16 '24 16:12 phannebohm

In the meantime I opened #13420 to make it fail with a proper error message.

phannebohm avatar Dec 16 '24 17:12 phannebohm

@bilderbuchi, if you are familiar with Susan and our C runtime you can take care of that. Please notify me if you do.

Unfortunately, I am not, with neither. Thank you!

bilderbuchi avatar Dec 17 '24 09:12 bilderbuchi