Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix empty array slices in Cpp runtime #7649

Merged
merged 4 commits into from Jul 5, 2021
Merged

Fix empty array slices in Cpp runtime #7649

merged 4 commits into from Jul 5, 2021

Conversation

rfranke
Copy link
Member

@rfranke rfranke commented Jul 3, 2021

stop=0 in start:step:stop was used to mark end so far.
It is needed for empty slices though.

See e.g. ModelicaTest.Math.TestMatrices2b (MSL 4.0.0) raising

ERROR  : init  : SimManager: Wrong slice exceeding array size

`stop=0` in `start:step:stop` was used to mark `end` so far.
It is needed for empty slices though.

See e.g. ModelicaTest.Math.TestMatrices2b (MSL 4.0.0) raising
ERROR  : init  : SimManager: Wrong slice exceeding array size
@perost perost enabled auto-merge (squash) July 4, 2021 17:52
@rfranke
Copy link
Member Author

rfranke commented Jul 4, 2021

osx-fmuchecker hangs since an hour. I used this opportunity to extend the test case -- this only works since the merge of #7650 (initialization of reductions).

@perost perost merged commit 1ab95b2 into OpenModelica:master Jul 5, 2021
adrpo pushed a commit to adrpo/OpenModelica that referenced this pull request Jul 5, 2021
* Fix empty array slices in Cpp runtime

`stop=0` in `start:step:stop` was used to mark `end` so far.
It is needed for empty slices though.

See e.g. ModelicaTest.Math.TestMatrices2b (MSL 4.0.0) raising
ERROR  : init  : SimManager: Wrong slice exceeding array size

* Add missing access operators for 3-5 dims to ArraySliceConst

* Prevent reduction of ArraySlice for index set of size 1
adrpo added a commit that referenced this pull request Jul 6, 2021
* fix issues with OpenSuse (#7636)

- installs liblpsolve55 in lib64, fix the copy
- add the path to ffi include when building the Compile runtime to fix:
  ./../../3rdParty/libffi/install/include/ffi.h:58:10: error: 'ffitarget.h'
  file not found with <angled> include; use "quotes" instead

* Flattening fixes (#7626)

- Flatten types in statements.
- Flatten expressions after evaluating bindings in EvaluateConstants,
  since they come from the instance tree and not the flat model.
- Create typed bindings instead of flat bindings in some places where
  the generated binding might not actually be flat.
- Lift the result in evalComponentBinding if necessary to ensure it has
  the same dimensions as the cref being evaluated.
- Apply the array hack for dimensions in Dimension.fromExp instead of
  during typing to simplify problematic dimensions in more cases.
- Don't double delete components.

* Fix loading of subpackages in case of encryption (#7639)

* Fix loading of subpackages in case of encryption

* Return the class parts

* Expand calls with multiple iterators (#7642)

- Expand reductions/array constructors with multiple iterators into
  nested calls when generating the DAE structure, since the code
  generation can't handle multiple iterators.

* Simplify min/max reductions more (#7644)

- Evaluate min/max reductions if possible.
- Fix Expression.typeOf for relations, to avoid using the wrong default
  value when evaluating reductions.

* Fix product in old frontend (#7646)

- Don't expand product of an expression with unknown dimensions.

* Implement product operator and DIV_SCALAR_ARRAY in Cpp runtime (#7641)

* Implement product operator in Cpp runtime

* Update OMCompiler/SimulationRuntime/cpp/Core/Math/ArrayOperations.cpp

Co-authored-by: phannebohm <phannebohm@fh-bielefeld.de>

* Update test

* Implement DIV_SCALAR_ARRAY in Cpp runtime

See e.g. PowerSystems.Examples.AC3ph.Precalculation.EqCircFromTransDat

Co-authored-by: phannebohm <phannebohm@fh-bielefeld.de>

* Implement const element access in DynArrayDim3 (#7647)

See e.g. PowerSystems.Examples.AC3ph.Precalculation.EqCircFromTransDat

* Fix Cpp writeLhsCref to not use ArraySliceConst on lhs (#7648)

* Fix Cpp writeLhsCref to not use ArraySliceConst on lhs

Call 'daeExpCref(isLhs=true, ...)' instead of 'daeExpCrefRhs(...)' in scalarLhsCref.

See e.g. PowerSystems.Examples.AC3ph.Precalculation.TransDatFromEqCirc raising:
ERROR  : init  : SimManager: Can't assign array to ArraySliceConst

* Whitespaces

* Fix initialization of reductions in Cpp runtime (#7650)

* Fix initialization of reductions in Cpp and C runtime

`foldExpPre` had been declared but not used. This resulted in Cpp errors.

See e.g. ModelicaTest.Fluid.TestComponents.Machines.TestWaterPumpCharacteristics
or PowerSystems.Examples.AC3ph.Precalculation.Z_matrixTrDat3
ERROR  : init  : SimManager: Right and left array must have the same size for element wise addition

* Revert fix of initialization of reductions for C runtime

* Don't dump equations into Cpp FMU sources (#7643)

* Fix empty array slices in Cpp runtime (#7649)

* Fix empty array slices in Cpp runtime

`stop=0` in `start:step:stop` was used to mark `end` so far.
It is needed for empty slices though.

See e.g. ModelicaTest.Math.TestMatrices2b (MSL 4.0.0) raising
ERROR  : init  : SimManager: Wrong slice exceeding array size

* Add missing access operators for 3-5 dims to ArraySliceConst

* Prevent reduction of ArraySlice for index set of size 1

* Fix unlifting of array types in ffi runtime (#7638)

Co-authored-by: perost <perost86@gmail.com>
Co-authored-by: Adeel Asghar <adeel.asghar@liu.se>
Co-authored-by: rfranke <rfranke@users.noreply.github.com>
Co-authored-by: phannebohm <phannebohm@fh-bielefeld.de>
@casella
Copy link
Contributor

casella commented Jul 7, 2021

@rfranke, with this commit the results of PowerSystems_cpp became better than those of PowerSystems with the C runtime, 99 vs 94 successful simulations out of 118 models.

I guess this is also due to the fact that PowerSystems_cpp is now testing "version": "1.0.1-master" of the library, while PowerSystems is still testing "version": "1.0.0". This is also confirmed by the fact that the models failing with the C runtime are actually failing in the backend already.

@sjoelund, @adrpo, would you mind updating the test script for the C runtime to also use "version": "1.0.1-master" ?

Thanks!

@casella
Copy link
Contributor

casella commented Jul 7, 2021

OK, so we have the following branches tested with the master version of OMC

  • PowerSystems, running on 1.0.0, standard C runtime
  • PowerSystems_cpp, running on 1.0.1-master, cpp runtime
  • PowerSystems_latest, running on 1.0.1-master, C runtime
  • PowerSystems_master, running on 1.0.0 until 16 June 2021, currently not running
  • PowerSystems_fmi-cpp, not running since 2017
  • PowerSystems_fmi, not running since 2017
  • PowerSystems_fmi cpp, not running since 2017 (this one has a space instead of a dash between fmi and cpp-9

If this is not a mess, I don't know what is 😄

May I at least suggest that we delete the obsolete directories, for which new tests are no longer generated, or at least that we move them in a discontinued directory? I don't really see the point of keeping old results obtained with old versions of OMC and old versions of PowerSystems, frozen forever at some arbitrary point in time, showing lots of issues that will never go away.

@rfranke, what do you think?

For the renaming, we need to completely overhaul the whole thing, see #5276, I'll try to take care of that this autumn.

@rfranke rfranke deleted the emptyslice branch July 7, 2021 10:31
@rfranke
Copy link
Member Author

rfranke commented Jul 7, 2021

It's fine for me as it is -- as we decided to package the latest version with OpenModelica. The printed library version 1.0.1 makes clear that PowerSystems_cpp should not be related to PowerSystems above, but to PowerSystems_latest below.
Still 99 vs 98 :)

@sjoelund
Copy link
Member

sjoelund commented Jul 7, 2021

OK, so we have the following branches tested with the master version of OMC

Libraries listed there are not the ones tested. Those are found in the overview reports, and the directory has a lot of old stuff in them.

@casella
Copy link
Contributor

casella commented Jul 7, 2021

Libraries listed there are not the ones tested.

That is not immediately apparent if you happen to land on that page.

Those are found in the overview reports

OK, I guess I should start from there instead

and the directory has a lot of old stuff in them.

That's my point, I would suggest to clean it up from obsolet stuff.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants