Evaluation of new OpenSourceTables

[modelica-trac-importer]

Reported by beutlich on 9 Apr 2013 14:38 UTC
According to the latest development request to implement open source tables for

Modelica.Blocks.Sources.CombiTimeTable
Modelica.Blocks.Tables.CombiTable1D
Modelica.Blocks.Tables.CombiTable1Ds
Modelica.Blocks.Tables.CombiTable2D

ITI is currently working on it. Together with the MA board we decided to provided a pre-release for early and wide evaluation.

With r6216 I committed the current project to [source:/Modelica/branches/development/OpenSourceTables OpenSourceTables].

So far I did not commit any binaries of the external library but prepared Makefiles and Visual Studio project files for self-compilation:

• The MinGW-Makefile can be used for compilation and usage with OpenModelica.
• The MSVC LIB Release configuration builds a static library (*.lib) with /MT and can therefore be used with Dymola.
• The MSVC DLL Release configuration uses SimulationX-specific ModelicaUtilities and builds a dynamic link library (*.dll). It can therefore be used with SimulationX.

Please evaluate code and performance thoroughly! Use this ticket for your comments. Any feedback is appreciated. In order to not postpone MSL 3.2.1 release any longer rapid checks are welcome. I am afraid I cannot consider changes or wishes after 04/25/13.

TODO:

• Finish the test models for the three Tables models.
• More compiler tests.

This ticket addresses/fixes #129, #338, #628, #1022, #1028.

---

Migrated-From: https://trac.modelica.org/Modelica/ticket/1067

[modelica-trac-importer]

Comment by sjoelund.se on 10 Apr 2013 15:03 UTC
I'll start with the obvious - there are no Makefile targets for Linux and very, very poor build instructions.
MinGW works, but there are no targets for compilation against matlab/hdf5 files, and there is no zlib target for those who have it precompiled?
The 3rd-party sources used should be in the svn, or the targets should use system libraries.
Since the design seems to rely on no system libraries being used (they are not used in the Modelica annotations), these libraries should be included and the makefiles should call them in order to always go back to the same version as before. This is probably especially important since you used actual sources of libmatio anyway. Better make sure everything comes from the same version of the library...
-DHDF5 and -DMAT73 do not yield working executables unless used together, so only one should remain.

CTT.Test0 is partial but marked as an experiment. The other models are not marked partial but are not marked as experiments (you do not inherit annotations unless they say so, right?).

The problems with the tests is that they cannot be run without a working (old) Tables implementation. And once the old tables are replaced, there are no more tests to run, right (so put the old functions under NewTables.Test.OldTables)?
I would suggest structuring the tests so it is apparent which ones require old tables and which do not.

fileName = classDirectory() + "Data/test_v4.mat" - classDirectory() was never part of any Modelica standard. ModelicaServices has support for converting modelica:// URIs to filenames.

I could get a few models to run using setLinkerFlags("-lmatio -lhdf5 " + getLinkerFlags()); and the attached Makefile.

It is not quite there yet. There are very many I cannot test due to classDirectory() issues, etc. So there is probably more. Fix these first easy points and I will try again.

[modelica-trac-importer]

Modified by sjoelund.se on 10 Apr 2013 15:06 UTC

[modelica-trac-importer]

Comment by beutlich on 10 Apr 2013 15:38 UTC
Thanks for first feedback.

Yes and sorry, there is little Linux support here. I have no clue which system libraries I can assume to be installed. Do you have matio and hdf5 as system libraries? I would not have thought of openmpi. So best is to compile it all by yourself but I was to lazy to add a hdf Makefile. This is surely not my task. You may skip CCT.Test57 and CCT.Test58 for now. You should also know that OpenModelica support is not part of the implementation contract.

You are right about double preprocessor macros -DHDF5 and -DMAT73 that only work if specified together. I will consider this.

You may also remove old table implementation from CCT.Test0 to run the tests.
You are also right about the tests and comparison with old tables. I had this already in mind. Main focus today was on compatibility with old tables. I did not want to use any post-processor tools for comparison so I put both implementations in one model.

I can change the experiment annotations of the test models of course.

For now, you can manually replace classDirectory() by the table path in the files using search + replace.

[modelica-trac-importer]

Comment by beutlich on 10 Apr 2013 15:47 UTC
I forgot to ask why you link against libmatio.a? That should not be necessary and is also wrong since ModelicaMatIO.c baically is a patched libmatio tailored to Modelica environment.

I did not add hdf5, zlib and szip sources by purpose but stated the download links. I wanted to keep it clean and simple.

[modelica-trac-importer]

Comment by sjoelund.se on 10 Apr 2013 15:48 UTC
Replying to [comment:3 beutlich]:

Yes and sorry, there is little Linux support here. I have no clue which system libraries I can assume to be installed. Do you have matio and hdf5 as system libraries? I would not have thought of openmpi. So best is to compile it all by yourself but I was to lazy to add a hdf Makefile. This is surely not my task. You may skip CCT.Test57 and CCT.Test58 for now. You should also know that OpenModelica support is not part of the implementation contract.

The main problem is even if we have the libraries installed, someone else might not. And yes, there are 3 different versions of hdf5 only on Ubuntu Linux. They all require different flags to run. And the libraries have to be in the Modelica annotation.

The best way would be to include them in the .a-file like you do on Windows. All this needs is compiling the sources. Which means the sources for of the hd5 code (as well as libmatio) should be provided in the svn. If they are there I can compile the same version on all platforms and reduce the burden of maintenance.

This is the common way to do it anyway - either as svn externals or actually in the svn repository. Then you can always checkout an old version of the tables and get the same library you used at the time.

And yes, I can replace everything classDirectory() manually, but wouldn't all tool vendors need to do this?

[modelica-trac-importer]

Comment by sjoelund.se on 10 Apr 2013 15:54 UTC
Replying to [comment:4 beutlich]:

I forgot to ask why you link against libmatio.a? That should not be necessary and is also wrong since ModelicaMatIO.c baically is a patched libmatio tailored to Modelica environment.

I did not add hdf5, zlib and szip sources by purpose but stated the download links. I wanted to keep it clean and simple.

svn propedit svn:externals
Add for example: https://github.com/madler/zlib/tags/v1.2.7.1 zlib

And the full source is automatically added (and can be disabled for users who prefer that). Then it is much easier to create Makefile targets to compile everything :)

As for libmatio... You are right, I might have been wrong since I possibly used the wrong defines and got a bad library for some reason. I had to add it before, but don't now.

[modelica-trac-importer]

Comment by beutlich on 10 Apr 2013 15:58 UTC
Actually we should not focus on the library dependencies. Project should run stand-alone without hdf5, zlib. Implementation contract is only about v4 MAT-files which do not require them. v6 MAT-files come for free anyway then. And v7 and v7.3 is only nice to have but certainly not the most important feature.

[modelica-trac-importer]

Comment by choeger on 10 Apr 2013 18:24 UTC
The best way would definetely be to use a good build system first. Handling dependencies with a sole makefile is tedious and error prone. I suggest autoconf or cmake. Both work on windows (albeit cmake is much easier there).

[modelica-trac-importer]

Comment by sjoelund.se on 10 Apr 2013 18:47 UTC
Christoph, I agree. Especially for Mac OSX it would be a pain to get all hdf stuff, etc working without a good build system. (Everything is always harder on Mac). I know how to build something that would sort of work on Linux (make a shared object instead of the static one built by default). But I know from experience this never works the way you would expect on OSX.

I would not say the build system is the main priority though. Getting it written in standard Modelica should be !#1. I can always donate a sane build system for MA once we start to maintain it as part of MSL. I would have to implement one for OpenModelica anyway.

I tested the library again without MAT73 or HDF5 and yes MATv4 still works, so I guess that is fine as long as it is not hard to enable the support manually. A good build system would still be nice to set CFLAGS, CPPFLAGS, LDFLAGS, enable/disable features, do feature tests, and so on.

And as a sidenote, autoconf is much nicer than cmake; everyone who writes something in cmake always requires the latest version because the old versions did not have everything they need. And of course not everyone has access to the latest cmake (for example, the latest Debian release still does not have cmake 2.8.6 which FMIL requires).

[modelica-trac-importer]

Comment by sjoelund.se on 10 Apr 2013 18:57 UTC
Also, why does everything always have to be password-protected? The ticket is not hidden, but most of our developers would not have access to test the library. My normal workflow is to just make it an svn external, setup build scripts inside OpenModelica, then ask people to try it out. Saves me the trouble of testing Windows versions.

[modelica-trac-importer]

Comment by beutlich on 10 Apr 2013 19:40 UTC
Replying to [comment:10 sjoelund.se]:

Also, why does everything always have to be password-protected? The ticket is not hidden, but most of our developers would not have access to test the library. My normal workflow is to just make it an svn external, setup build scripts inside OpenModelica, then ask people to try it out. Saves me the trouble of testing Windows versions.

I did not know at my initial commit that is a non-public repository. I could attach the zipped sources but this would not help you here. Maybe the admin can change read access rights.

[modelica-trac-importer]

Comment by sjoelund.se on 10 Apr 2013 20:13 UTC
Minor issue:

OpenSourceTables/Modelica/NewTables/Resources/C-Sources/ModelicaTables.c: In function ‘readTxtTable’:
OpenSourceTables/Modelica/NewTables/Resources/C-Sources/ModelicaTables.c:2752: warning: format ‘%u’ expects type ‘unsigned int *’, but argument 3 has type ‘size_t *’
OpenSourceTables/Modelica/NewTables/Resources/C-Sources/ModelicaTables.c:2759: warning: format ‘%u’ expects type ‘unsigned int *’, but argument 3 has type ‘size_t *’

Also tons of unused results from fread, probably in libmatio. I suppressed them.

[modelica-trac-importer]

Comment by sjoelund.se on 11 Apr 2013 07:25 UTC
Replying to [comment:9 sjoelund.se]:

And as a sidenote, autoconf is much nicer than cmake

Feel free to use the OM autoconf diffs (or the relevant part of om:source:trunk/configure.in): om:r15784 om:r15785
It seems like a reasonable way to build a dynamic library (since it is dynamic all dependencies are added). I could possibly update it a little to link zlib but not hdf5, but for just v7 MAT I didn't feel like it even though it's a small change.

[modelica-trac-importer]

Modified by beutlich on 11 Apr 2013 07:33 UTC

[modelica-trac-importer]

Comment by beutlich on 11 Apr 2013 07:46 UTC
Feel free to add/update the configuration script or Makefile.

[modelica-trac-importer]

Comment by sjoelund.se on 11 Apr 2013 09:58 UTC
I will add an autoconf target then (maybe today).

For now, this was the result of automatic testing with MSL 3.2.1 and the latest OpenModelica: http://dev.openmodelica.org/~marsj/testing/BuildModelRecursive.html
(I will update this as more commits are made and problems are fixed)

[modelica-trac-importer]

Comment by sjoelund.se on 11 Apr 2013 14:34 UTC
I added a directory using autotools under source:Modelica/branches/development/OpenSourceTables/unix/. We tested it on Linux (dynamic,zlib,hdf5), OSX (dynamic, zlib, no hdf5) and MinGW (static, no zlib or hdf5). So I am reasonably happy.

Now I'm just waiting for classDirectory() and experiment annotations.

[modelica-trac-importer]

Comment by beutlich on 11 Apr 2013 15:07 UTC
Replying to [comment:12 sjoelund.se]:

Minor issue:

OpenSourceTables/Modelica/NewTables/Resources/C-Sources/ModelicaTables.c: In function ‘readTxtTable’:
OpenSourceTables/Modelica/NewTables/Resources/C-Sources/ModelicaTables.c:2752: warning: format ‘%u’ expects type ‘unsigned int \*’, but argument 3 has type ‘size_t \*’
OpenSourceTables/Modelica/NewTables/Resources/C-Sources/ModelicaTables.c:2759: warning: format ‘%u’ expects type ‘unsigned int \*’, but argument 3 has type ‘size_t \*’

Thanks. Fixed by r6235.

[modelica-trac-importer]

Comment by sjoelund.se on 11 Apr 2013 15:31 UTC
It is fixed, but only for platforms where sizeof(long)==sizeof(size_t).
sscanf(token, "%lu", (unsigned long*)nRow) looks quite dangerous to me. You never want to sscanf into something of type size_t since the size of that type is not well-defined.

[modelica-trac-importer]

Comment by beutlich on 11 Apr 2013 15:39 UTC
Thatswhy C99 introduced %z format specifier which I cannot use for compatibility reasons.

[modelica-trac-importer]

Comment by beutlich on 11 Apr 2013 15:54 UTC
I cannot solve the other problem with classDirectory(). If replaced by standard conform funtion Modelica.Utilities.Files.loadResource() it does not work with default ModelicaServices 3.2.1.

[modelica-trac-importer]

Comment by otter on 11 Apr 2013 16:01 UTC
Replying to [comment:1 sjoelund.se]:

fileName = classDirectory() + "Data/test_v4.mat" - classDirectory() was never part of any Modelica standard. ModelicaServices has support for converting modelica:// URIs to filenames.

In the trunk of MSL there is now a function:

fileName = Modelica.Utilities.Files.loadResource(uri)

that returns the file name of an uri (such as uri = "modelica://NewTables.Test/Data/test.txt"). This function is implemented with a new ModelicaServices function "ModelicaServices.[wiki:ExternalReferences].loadResource". In Dymola this ModelicaServices function is implemented with function "Dymola_ResolveURI" that existed in previous Dymola releases. I have just committed this updated ModelicaServices for Dymola and stored it as !https://svn.modelica.org/projects/Modelica/branches/development/OpenSourceTables/Modelica/ModelicaServices-Variants/Dymola/ModelicaServices 3.2.1

Therefore, you can replace classDirectory with loadResource and use this ModelicaServices for Dymola.

[modelica-trac-importer]

Comment by sjoelund.se on 11 Apr 2013 16:02 UTC
Replying to [comment:20 beutlich]:

Thatswhy C99 introduced %z format specifier which I cannot use for compatibility reasons.

So scanf into an unsigned long which you use to assign to the size_t. Or create a utility function that scans a size_t. Inline it if you are worried about performance.

Replying to [comment:21 beutlich]:

I cannot solve the other problem with classDirectory(). If replaced by standard conform funtion Modelica.Utilities.Files.loadResource() it does not work with default ModelicaServices 3.2.1.

Then you have a problem. classDirectory() is for sure not in the specification. I would just assume the vendors have implemented loadResource in their Services.

[modelica-trac-importer]

Comment by otter on 11 Apr 2013 16:04 UTC
Replying to [comment:21 beutlich]:

I cannot solve the other problem with classDirectory(). If replaced by standard conform funtion Modelica.Utilities.Files.loadResource() it does not work with default ModelicaServices 3.2.1.

Use the just committed ModelicaServices for Dymola under
OpenSourceTables/Modelica/ModelicaServices-Variants/Dymola/ModelicaServices 3.2.1

and loadResource should be supported in Dymola

[modelica-trac-importer]

Comment by beutlich on 11 Apr 2013 19:41 UTC
Replying to [comment:19 sjoelund.se]:

It is fixed, but only for platforms where sizeof(long)==sizeof(size_t).
sscanf(token, "%lu", (unsigned long*)nRow) looks quite dangerous to me. You never want to sscanf into something of type size_t since the size of that type is not well-defined.

Yes, that was dangerous. It should be finally fixed by r6240.

[modelica-trac-importer]

Comment by sjoelund.se on 11 Apr 2013 20:47 UTC
OK, I have one more comment. The error tolerances in some of the tests (like Test14) seem almost insane to me. I had to set the dassl tolerance to 10e-12 (OM default is 10e-6) in order to get the event at exactly the same time as whatever tool generated the mat-file (Dymola?). Fixed step RK4 was fine regardless.
It makes it very hard to know if there is something wrong with a test due to table comparison not working correctly (what I assume should be tested) or if it fails due to the tolerances being too tight.

[modelica-trac-importer]

Comment by beutlich on 12 Apr 2013 07:01 UTC
Replying to [comment:26 sjoelund.se]:

OK, I have one more comment. The error tolerances in some of the tests (like Test14) seem almost insane to me. I had to set the dassl tolerance to 10e-12 (OM default is 10e-6) in order to get the event at exactly the same time as whatever tool generated the mat-file (Dymola?). Fixed step RK4 was fine regardless.
It makes it very hard to know if there is something wrong with a test due to table comparison not working correctly (what I assume should be tested) or if it fails due to the tolerances being too tight.

That is exactly the case under test. In SimulationX I can increase the solver tolerance or max. solver step size arbitrarily and the time event is always exactly met. Thus I guess there is a problem with event handling in OM.
MAT-files are generated by MATLAB R2012b.

[modelica-trac-importer]

Comment by beutlich on 12 Apr 2013 07:17 UTC
Replying to [comment:24 otter]:

Use the just committed ModelicaServices for Dymola under
OpenSourceTables/Modelica/ModelicaServices-Variants/Dymola/ModelicaServices 3.2.1

and loadResource should be supported in Dymola

By r6242 I removed calls to classDirectory() though Dymola 2013 FD01 then fails with

Compiling and linking the model (Visual C++). 
 
Setting environment for using Microsoft Visual Studio 2008 x86 tools.
dsmodel.c
dsmodel.c(64) : warning C4013: 'Dymola_ResolveURI' undefined; assuming extern returning int
dsmodel.c(64) : warning C4047: '=' : 'const char *' differs in levels of indirection from 'int'
dsmodel.c(70) : warning C4090: 'function' : different 'const' qualifiers
dsmodel.c(436) : warning C4244: 'function' : conversion from 'double' to 'int', possible loss of data
dsmodel.c(437) : warning C4244: 'function' : conversion from 'double' to 'int', possible loss of data
   Creating library dymosim.lib and object dymosim.exp
dsmodel.obj : error LNK2019: unresolved external symbol _Dymola_ResolveURI referenced in function _Modelica_Utilities_Files_loadResource
dymosim.exe : fatal error LNK1120: 1 unresolved externals
 
Error generating Dymosim. 

[modelica-trac-importer]

Comment by sjoelund.se on 12 Apr 2013 07:47 UTC
Replying to [comment:27 beutlich]:

In SimulationX I can increase the solver tolerance or max. solver step size arbitrarily and the time event is always exactly met. Thus I guess there is a problem with event handling in OM.

It is not really wrong. It just uses a numerical method since the trapezoid has a time event OM cannot optimize. The trapezoid expects I find time 1.5 (why the trapezoid block does not search for the exact time of the event using sampling and integer arithmetic is beyond me, but that is another matter... it will just skew a little if you have many periods).

I do find the event at time 1.500000000000001, which is the limit of double precision (searching for the state event). That is with tolerance set to 1e-12. At this time I have a slight error of 1e-15 between the table and trapezoid, which is fine (the integrated error goes to 1e-12 from 1e-31).
Tolerance set to 1e-6, I find the event at time 1.500000000000005 with an error of 5e-15, which is too high for the model (bumps the integrated error up to 7.7e-9 from 1e-31 due to the feedback, etc; the limit is set to 1e-10). But clearly dassl should accept it since it is a tiny error relative to signal...
Using tables together with models with zero-crossings is bad. If the trapezoid was modeled using time events that were easier to detect (sampling), it would be fine. Or if the tolerance hard-coded in the model wasn't set so high.

Just saying some of these models are very, very, dependent on what symbolic optimizations are performed by the tool and what kind of solver is used (rungekutta without root finding has a lower error than dassl in this test...).

[modelica-trac-importer]

Comment by sjoelund.se on 12 Apr 2013 08:19 UTC
OK, I looked closer at the implementation of the time table. It really does require you to find the exact time point for the event if you want to pass these tests. Or instead of like us the point exactly after the zero-crossing you find the point right before the zero-crossing instead.

[modelica-trac-importer]

Comment by otter on 12 Apr 2013 09:09 UTC
Replying to [comment:28 beutlich]:

Replying to [comment:24 otter]:

Use the just committed ModelicaServices for Dymola under
OpenSourceTables/Modelica/ModelicaServices-Variants/Dymola/ModelicaServices 3.2.1

and loadResource should be supported in Dymola

By r6242 I removed calls to classDirectory() though Dymola 2013 FD01 then fails with

Strange: I tried with Dymola 2013 FD01, and for me it works. From the error message it seems that the right library is loaded since function Dymola_ResolveURI is called (but maybe you double check by clicking on the Info layer of package ModelicaServices and check the file name at the bottom of the info), but linking fails. If linking fails, it might be that for some (strange) reasons the compiler links to a library from another Dymola version.

[modelica-trac-importer]

Comment by beutlich on 12 Apr 2013 09:26 UTC
Which lib/obj is Dymola_ResolveURI implemented in? I searched whole program folder for "ResolveURI" and see only these reults

C:\Programme\Dymola 2013 FD01\bin\Dymola.exe
C:\Programme\Dymola 2013 FD01\bin64\Dymola.exe
C:\Programme\Dymola 2013 FD01\Modelica\Library\ModelicaServices 1.2\package.mo

[modelica-trac-importer]

Comment by hansolsson on 12 Apr 2013 09:40 UTC
Replying to [comment:32 beutlich]:

Which lib is Dymola_ResolveURI implemented? I searched whole program folder for "ResolveURI" and see only these reults

C:\Programme\Dymola 2013 FD01\bin\Dymola.exe
C:\Programme\Dymola 2013 FD01\bin64\Dymola.exe
C:\Programme\Dymola 2013 FD01\Modelica\Library\ModelicaServices 1.2\package.mo

There are restrictions on it that makes it unsuitable for a lib (although I could not find the problematic case in the Modelica-package):

It can only resolve constant URIs; i.e. similarly as classDirectory it can handle ResolveURI("modelica://NewTables.Test/data4.mat") - but it cannot currently handle ResolveURI(a+b); unless a+b are string constants.

If it fails for a string constant it must be that the string constant is not considered as constant - I cannot see why that would happen.

If the data is stored in a data-base translation might also have to project the data to file - and then return the file-name.
(And yes, will add a better diagnostic for this.)

[modelica-trac-importer]

Comment by otter on 12 Apr 2013 10:03 UTC
Replying to [comment:32 beutlich]:

Which lib/obj is Dymola_ResolveURI implemented in? I searched whole program folder for "ResolveURI" and see only these reults

C:\Programme\Dymola 2013 FD01\bin\Dymola.exe
C:\Programme\Dymola 2013 FD01\bin64\Dymola.exe
C:\Programme\Dymola 2013 FD01\Modelica\Library\ModelicaServices 1.2\package.mo

Please, try in the command window of Dymola 2013 FD01 the following command:

Modelica.Utilities.Files.loadResource("modelica://NewTables.Test/data/test.txt");

If this works, then this indicates that loadResource is supported in this Dymola version. In case of error it shows a possible reason (e.g. because wrong Modelica package was loaded).

[modelica-trac-importer]

Comment by beutlich on 12 Apr 2013 10:57 UTC
Replying to [comment:33 hansolsson]:

If it fails for a string constant it must be that the string constant is not considered as constant - I cannot see why that would happen.

I figured it out (see e.g. changes by r6244 in NewTables.Test.CombiTable2D.Test6): If the fileName is passed directly as constructor argument to the CombiTable2D compilation fails. But using an auxiliary parameter String it works.

Now it is time to test the real thing like e.g. comment:30 started with.

[modelica-trac-importer]

Comment by beutlich on 15 Apr 2013 20:20 UTC
I got the compilation test results from dSPACE: There are no compiler warnings or errors on dSPACE targets DS1005, DS1401 and SCALEXIO. For DS1006 there are 17 warnings of the kind warning: duplicate const'` in ModelicaTables.c for C-expressions like max(max(x,y),z). These warnings can be set silent by GCC compiler option -std=gnu99.

Beside dSPACE target compilers I also successfully checked MinGW (GCC 4.4.0 and GCC 4.7.2), Cygwin (GCC 4.3.0), OpenWatCom 1.3, LCC 2.x, Borland BCC 5.5 and MS compilers (VC6 and >= VS2005 (Win32 and x64)).

[modelica-trac-importer]

Comment by beutlich on 15 Apr 2013 20:27 UTC
Replying to [comment:1 sjoelund.se]:

The problems with the tests is that they cannot be run without a working (old) Tables implementation. And once the old tables are replaced, there are no more tests to run, right (so put the old functions under NewTables.Test.OldTables)?
I would suggest structuring the tests so it is apparent which ones require old tables and which do not.

I am not satisfied with that situation, too. I used the test models as my own test suite to check compatibility with old tables implementation. What I could do now is to remove all references to old table implementation (and also the runtime assertion). This allows stand-alone simulation of the new tables (however without compatibility check) now and also after the merge.

In the implementation contract it is said that there is a reference result for every test model. I could easily commit them (using SimulationX as Modelica tool).

Please advise. Thanks.

[modelica-trac-importer]

Comment by otter on 16 Apr 2013 07:10 UTC
Replying to [comment:37 beutlich]:

Replying to [comment:1 sjoelund.se]:

The problems with the tests is that they cannot be run without a working (old) Tables implementation. And once the old tables are replaced, there are no more tests to run, right (so put the old functions under NewTables.Test.[wiki:OldTables])?
I would suggest structuring the tests so it is apparent which ones require old tables and which do not.

I am not satisfied with that situation, too. I used the test models as my own test suite to check compatibility with old tables implementation. What I could do now is to remove all references to old table implementation (and also the runtime assertion). This allows stand-alone simulation of the new tables (however without compatibility check) now and also after the merge.

Please, keep the current tests, because they are very useful for any tool vendor to quickly check its implementation with the new implementation.

What would be additionally helpful is, as you proposed, to only have tests with the table blocks from Modelica.Blocks. These tests should be stored in ModelicaTest of the

• maintenance branch (!https://svn.modelica.org/projects/Modelica/branches/maintenance/3.2)
• and in the trunk (https://svn.modelica.org/projects/Modelica/trunk)

As a result, the test models of the maintenance branch provide results of the vendor specific implementation of 3.2 and the test models in the trunk provide results of the new table implementation. Some tools (such as Dymola) can then make regression testing and can automatically  test the differences between the two implementations. Test models of new table features for MSL 3.2.1 should be, of course, only be stored in the trunk.

In the implementation contract it is said that there is a reference result for every test model. I could easily commit them (using SimulationX as Modelica tool).

Yes, please just store in one directory csv (ASCII) references files for SimulationX. Please, use the same format as sketched in the Call for a csv-file comparison (same format as the result csv file from the FMU compliance checker: comma separated values; first row are the variable names, first column is time with name “time”, time values must be monotonically increasing; two identical time values that follow each other signal an event instant).

[modelica-trac-importer]

Comment by beutlich on 26 Apr 2013 08:29 UTC
Thanks to all evaluators for their feedback and contribution. All comments were considered.