Latest ModelicaExternalC breaks MSL 3.2.1

[modelica-trac-importer]

Reported by sjoelund.se on 27 Feb 2016 20:31 UTC
The new modularized ModelicaStandardTables breaks compatibility with MSL 3.2.1, which is particularly bad since MSL aims to be backwards compatible and the external C sources are backwards compatible at least back to MSL 2.x (and I think earlier). It would be very annoying if we need to start shipping multiple versions of ModelicaExternalC to support old MSL versions.

---

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

[modelica-trac-importer]

Comment by otter on 28 Feb 2016 09:05 UTC
Replying to [ticket:1928 sjoelund.se]:

The new modularized ModelicaStandardTables breaks compatibility with MSL 3.2.1, which is particularly bad since MSL aims to be backwards compatible and the external C sources are backwards compatible at least back to MSL 2.x (and I think earlier). It would be very annoying if we need to start shipping multiple versions of ModelicaExternalC to support old MSL versions.

Lets first summarize the current difference:

MSL 3.2.l:
  - ModelicaStandardTables (.lib, .dll, .a, .so, depending on tool and OS) containing:
    ModelicaStandardTables.c
    ModelicaMatIO.c
    zlib/*.c

MSL 3.2.2 (trunk):
  - ModelicaIO (.lib, .dll, .a, .so, depending on tool and OS) containing:
    ModelicaIO.c

  - ModelicaMatIO (.lib, .dll, .a, .so, depending on tool and OS) containing:
    ModelicaMatIO.c

  - ModelicaStandardTables (.lib, .dll, .a, .so, depending on tool and OS) containing:
    ModelicaStandardTables.c

  - zlib (.lib, .dll, .a, .so, depending on tool and OS) containing:
    zlib/*.c

The reason for the restructuring was the introduction of the new functions to write matrices in various Matlab binary formats and to read matrices in various Matlab binary formats. The C-code of this new functionality was included in ModelicaIO.c (with the plan to introduce more read/write functions in future releases, e.g., to read/write Excel binary files). Since ModelicaIO.c needs ModelicaMatIO.c and zlib/*.c, and these C-functions are unrelated to ModelicaStandardTables.c, they have been stored in separate libraries.

I was not aware of the issues that this could cause for OpenModelica. In Dymola, and as I understand this is the same in SimulationX, these object libraries are shipped separately for every release. To me, this is "cleaner" because a new MSL release (or also a maintenance release) by sure does not have any influence on a previous MSL release (like 3.2.1). If the same object libraries are always shipped with a tool, then suddenly a new MSL release (3.2.2) also influences a previous release (3.2.1).

Anyway, if a tool vendor prefers this approach for practical reasons, then MSL should support this view, if reasonably possible. In this case there seems to be two options (and both are fine with me):

Option 1:

  - ModelicaStandardTables (.lib, .dll, .a, .so, depending on tool and OS) containing:
    ModelicaStandardTables.c
    ModelicaMatIO.c
    zlib/*.c
    
  - ModelicaIO (.lib, .dll, .a, .so, depending on tool and OS) containing:
    ModelicaIO.c

This means that ModelicaStandardTables must be used in the Library annotation, when ModelicaIO is used in this library annotation.

Option 2:

  - ModelicaStandardTables (.lib, .dll, .a, .so, depending on tool and OS) containing:
    ModelicaStandardTables.c
    ModelicaMatIO.c
    zlib/*.c
    ModelicaIO.c

We could use either option 1 or option 2 and in a later MSL release restructure (and then clarify the consequences beforehand with the tool vendors), if the restructring has advantages.

[modelica-trac-importer]

Comment by sjoelund.se on 28 Feb 2016 09:16 UTC
Replying to [comment:1 otter]:

In Dymola, and as I understand this is the same in SimulationX, these object libraries are shipped separately for every release. To me, this is "cleaner" because a new MSL release (or also a maintenance release) by sure does not have any influence on a previous MSL release (like 3.2.1). If the same object libraries are always shipped with a tool, then suddenly a new MSL release (3.2.2) also influences a previous release (3.2.1).

This is wrong. At least my copy of Dymola comes with a single version of ModelicaExternalC which is used for both MSL 2.x and 3.x.

We could use either option 1 or option 2 and in a later MSL release restructure (and then clarify the consequences beforehand with the tool vendors), if the restructring has advantages.

Actually, we can't because MSL 3.2.1 already used option 2. It also does not make sense to refactor the libraries at this time since the IO parts are only used in Tables functions (you need both Library annotations at the same time always and never only use IO). It could have made sense if this was done for MSL 3.2.1.

[modelica-trac-importer]

Comment by otter on 28 Feb 2016 09:34 UTC
Replying to [comment:2 sjoelund.se]:

Replying to [comment:1 otter]:

In Dymola, and as I understand this is the same in SimulationX, these object libraries are shipped separately for every release. To me, this is "cleaner" because a new MSL release (or also a maintenance release) by sure does not have any influence on a previous MSL release (like 3.2.1). If the same object libraries are always shipped with a tool, then suddenly a new MSL release (3.2.2) also influences a previous release (3.2.1).

This is wrong. At least my copy of Dymola comes with a single version of ModelicaExternalC which is used for both MSL 2.x and 3.x.

You are right. I just concentrated on the issue with the tables and overlooked ModelicaExternalC. Shipping ModelicaExternalC with Dymola is also not a good solution (besides the issue above), because any change to this library requires that a user gets a new Dymola version and it is therefore very awkward to include new features in this library. Therefore, for the MSL version following 3.2.2, we should try to change this (so include the ModelicaExternalC object libraries in the MSL distribution).

We could use either option 1 or option 2 and in a later MSL release restructure (and then clarify the consequences beforehand with the tool vendors), if the restructring has advantages.

Actually, we can't because MSL 3.2.1 already used option 2. It also does not make sense to refactor the libraries at this time since the IO parts are only used in Tables functions (you need both Library annotations at the same time always and never only use IO). It could have made sense if this was done for MSL 3.2.1.

Hm. Don't misunderstand me. Option 2 is fine for me and we can just use it. However, option 1 should also be o.k., because the ModelicaStandardTables.lib/.a/.dll would be the same as in MSL 3.2.1 (with minor improvements), so practically no change, but there would be a new library, ModelicaIO.lib/.a/.dll that is only used in new models not present in MSL 3.2.1.

[modelica-trac-importer]

Comment by beutlich on 28 Feb 2016 12:14 UTC
Replying to [comment:3 otter]:

Therefore, for the MSL version following 3.2.2, we should try to change this (so include the ModelicaExternalC object libraries in the MSL distribution).

This would only work out if the tool-specific implementation of the ModelicaUtilities functions (ModelicaMessage etc.) are separated from the tool-independent modules of ModelicaExternalC.

For the dilemma with the libraries I see two other workarounds:
Option 3:

- ModelicaStandardTables (.lib, .dll, .a, .so, depending on tool)
  ../C-Sources/ModelicaStandardTables.c
  ../C-Sources/ModelicaMatIO.c
  ../C-Sources/zlib/*.c

- ModelicaIO (.lib, .dll, .a, .so, depending on tool and OS):
  ../C-Sources/ModelicaIO.c

- ModelicaMatIO (.lib, .dll, .a, .so, depending on tool and OS)
  ../C-Sources/ModelicaMatIO.c

- zlib (.lib, .dll, .a, .so, depending on tool and OS):
  ../C-Sources/zlib/*.c

Option 4:

- ModelicaStandardTables (.lib, .dll, .a, .so, depending on tool)
  ../C-Sources/ModelicaStandardTables.c
  ../C-Sources/ModelicaMatIO.c
  ../C-Sources/zlib/*.c

- ModelicaIO (.lib, .dll, .a, .so, depending on tool and OS):
  ../C-Sources/ModelicaIO.c
  ../C-Sources/ModelicaMatIO.c
  ../C-Sources/zlib/*.c

In both cases ModelicaMatIO and zlib are provided twice.

[modelica-trac-importer]

Comment by otter on 28 Feb 2016 12:18 UTC
I am a bit lost. What are the disdvantages of option 2 (or why would this not work)?

[modelica-trac-importer]

Comment by beutlich on 28 Feb 2016 12:44 UTC
Option 2 adds ModelicaIO.c to ModelicaStandardTables which does not really belong there. I'd prefer to keep tables implementation and file i/o implementaion separated even if they depend on common libs ModelicaMatIO, zlib and (optionally hdf5).

[modelica-trac-importer]

Comment by sjoelund.se on 28 Feb 2016 13:43 UTC
Replying to [comment:4 beutlich]:

Option 3:
Option 4:

The disadvantage of 3 and 4 is that if you build a static version of the library, you can no longer link against two of the libraries at the same time (because of duplicate symbols).

[modelica-trac-importer]

Comment by otter on 28 Feb 2016 14:15 UTC
Replying to [comment:6 beutlich]:

Option 2 adds ModelicaIO.c to ModelicaStandardTables which does not really belong there. I'd prefer to keep tables implementation and file i/o implementaion separated even if they depend on common libs ModelicaMatIO, zlib and (optionally hdf5).

The only (tiny) issue is the naming of the library in option 2 (at least, I do not see other drawbacks). Otherwise, one can argue that all IO of matrices and interpolation in matrices read from file are in one library. Just because the name "ModelicaStandardTables" does not fit perfectly for all the functions in such a library seems to be not a big drawback. Option 3 and 4 give the potential of just other (very serious) drawbacks, if by design the same functions are present twice in different object libraries.

[modelica-trac-importer]

Comment by sjoelund.se on 29 Feb 2016 06:47 UTC
There is another option as well: Rename ModelicaStandardTables to ModelicaStandardTables2 and compile a 3.2.1-compatible ModelicaStandardTables as well. If they are dynamically linked they don't take up much space, but it would mean some work to get the projects to link these dll/so-files correctly (and there would be a lot of duplicated binaries).

[modelica-trac-importer]

Comment by beutlich on 29 Feb 2016 07:40 UTC
See #1856 for the corresponding issue where ModelicaStandardTables was modularized.

[modelica-trac-importer]

Comment by otter on 29 Feb 2016 08:56 UTC
A chat discussion of Thomas, Martin (S.), and Martin (O.) gave the following conclusion:

• Keep the library structuring for MSL 3.2.2 as it is currently on the trunk.
• Tools that cannot use the libraries stored under Resources, need to provide special build-scripts.
• Martin (S.) will evaluate how to implement such build-scripts for OpenModelica.
• Thomas will document this in Resources\C-Sources\readme.txt.
• Martin (O.) will update the release notes for MSL 3.2.2 (better explaining that this might be non-backwards compatible for a tool, with a link to the readme.txt file)

[modelica-trac-importer]

Comment by sjoelund.se on 29 Feb 2016 09:01 UTC
2c7f524 adds MatIO as a dependency for StandardTables. This should work well for shared objects and be backwards compatible with 3.2.1 (I will confirm that later). For static libraries, if a Modelica compiler wishes to ship only 1 set of MSL libraries, it is necessary that the compiler knows about this dependency and links in MatIO if StandardTables is in a library annotation (unless libtool is used, in which case the .la-files contain the dependencies for static libraries).

[modelica-trac-importer]

Comment by otter on 29 Feb 2016 09:26 UTC
Adapted the release notes in 9dd336a and f386037:

The ModelicaStandardTables object library (.lib, .dll, .a, .so, depending on tool) has been split into the libraries ModelicaStandardTables, ModelicaMatIO, zlib and the new object library ModelicaIO has been added.
For a tool vendor this can be a non-backwards compatible change if the same object libraries have been used in the past for different releases of package Modelica. In Resources/C-sources/_readme.txt the issue is explained in detailed and how to resolve it. For a user this might be a non-backwards compatible change if he/she implemented an own external C interface function to one of the functions in the ModelicaStandardTables, ModelicaMatIO or zlib libraries. In this case, the library annotation to these functions need to be adapted.

[modelica-trac-importer]

Comment by beutlich on 29 Feb 2016 13:18 UTC
sjoelund: I miss libModelicaMatIO_la_LIBADD = libzlib.la in the Makefile.

[modelica-trac-importer]

Comment by sjoelund.se on 29 Feb 2016 13:22 UTC
I guess it should be there, but zlib is an empty library when linked dynamically and -lz is already part of StandardTables, so it happens to work anyway.

[modelica-trac-importer]

Comment by beutlich on 29 Feb 2016 13:24 UTC
Please add it if it is formally correct.

[modelica-trac-importer]

Comment by sjoelund.se on 29 Feb 2016 13:26 UTC
Added in f6555ab.

[modelica-trac-importer]

Comment by beutlich on 29 Feb 2016 13:32 UTC
Hm, I'd have added it the following way:

if INCLUDEZLIB
libModelicaMatIO_la_LIBADD = libzlib.la
endif

since only ModelicaMatIO depends on zlib.

[modelica-trac-importer]

Comment by sjoelund.se on 29 Feb 2016 13:34 UTC
It will matter very little for MSL 3.2.2 since it always links MatIO StandardTables and zlib together. So it will all be there anyway.

[modelica-trac-importer]

Comment by beutlich on 29 Feb 2016 13:39 UTC
Replying to [comment:19 sjoelund.se]:

It will matter very little for MSL 3.2.2 since it always links MatIO StandardTables and zlib together. So it will all be there anyway.

Sorry I do not get it? I thought we speak about a modularization issue here. Can you show us the binary objects and dependencies that OM generates!

[modelica-trac-importer]

Comment by sjoelund.se on 29 Feb 2016 13:52 UTC
I'll change the build project to not link in z and hdf5 except for in ModelicaStandardTables. Or does MatIO also depend on z?

[modelica-trac-importer]

Comment by beutlich on 29 Feb 2016 13:54 UTC
Only MatIO optionally depends on zlib and hdf5: https://github.com/tbeu/matio#21-dependencies

ModelicaStandardTables depends on ModelicaMatIO. It does not directly depend on zlib (aka dependency chain).

[modelica-trac-importer]

Comment by sjoelund.se on 29 Feb 2016 14:38 UTC
fb20814 now handles all the dependencies I hope (ModelicaIO depends on ModelicaMatIO), and does not link anything unless it is needed.

[modelica-trac-importer]

Comment by beutlich on 29 Feb 2016 14:47 UTC
attachment:configure.ac.patch fixes some strings. Please apply the patch if it works for you.

[modelica-trac-importer]

Comment by sjoelund.se on 29 Feb 2016 14:51 UTC
Done in c45cf3d.

[modelica-trac-importer]

Comment by beutlich on 29 Feb 2016 15:28 UTC
I updated the readme by aefe8eb. If it is well documented you can close this ticket.

Thank for you valuable contribution.

[modelica-trac-importer]

Modified by sjoelund.se on 29 Feb 2016 15:32 UTC

[modelica-trac-importer]

Changelog modified by sjoelund.se on 29 Feb 2016 15:32 UTC
Clarified how tools should handle MSL 3.2.2 C-sources in order to maintain backwards compatibility with MSL 3.2.1.