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

Superfluous multiple inheritance in Modelica.Mechanics.MultiBody.Visualizers.Advanced.Shape #845

Closed
modelica-trac-importer opened this issue Jan 14, 2017 · 8 comments
Assignees
Labels
bug Critical/severe issue L: Mechanics.MultiBody Issue addresses Modelica.Mechanics.MultiBody worksforme Clarified issue
Milestone

Comments

@modelica-trac-importer
Copy link

Reported by anonymous on 1 Oct 2012 12:04 UTC
Model Modelica.Mechanics.MultiBody.Visualizers.Advanced.Shape extends both ModelicaServices.Animation.Shape and Modelica.Utilities.Internal.PartialModelicaServices.Animation.PartialShape where ModelicaServices.Animation.Shape itself extends Modelica.Utilities.Internal.PartialModelicaServices.Animation.PartialShape (in the default or Dymola implementation of Modelica Services). In this case there is no need that Modelica.Mechanics.MultiBody.Visualizers.Advanced.Shape extends Modelica.Utilities.Internal.PartialModelicaServices.Animation.PartialShape. It is all done in ModelicaServices.Animation.Shape.


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

@modelica-trac-importer modelica-trac-importer added this to the MSL3.2.1 milestone Jan 14, 2017
@modelica-trac-importer modelica-trac-importer added bug Critical/severe issue L: Mechanics.MultiBody Issue addresses Modelica.Mechanics.MultiBody labels Jan 14, 2017
@modelica-trac-importer
Copy link
Author

Comment by otter on 9 Dec 2012 15:57 UTC
This is the design, as agreed when introducing ModelicaServices:

  • The desired interface in ModelicaServices is defined under
    !Modelica.Utilities.Internal.PartialModelicaServices.
  • This interface is used both by library Modelica and by
    library ModelicaServices.
  • It is then guaranteed that at once an error occurs when
    the implementation in ModelicaServices does not agree to this interface

I agree that this multiple inheritance is not nice and might give complications to tools. It is fine for me to remove this, and make the approach a bit less safe, but simpler. The Modelica language group should decide. Changed the owner to Hans.

@modelica-trac-importer
Copy link
Author

Comment by hansolsson on 7 Jan 2013 08:47 UTC
Trying to think about this:

ModelicaServices can be implemented in other ways - one possibility is to make ModelicaServices self-contained (by duplicating - not inherting PartialModelicaServices). That allows e.g. MSL 3.2.1 to use ModelicaServices 3.2.2.
From a tool-perspective the benefit is that you only need to ship and maintain one ModelicaServices for multiple MSL-versions.

The multiple inheritance will detect if there is any error in this duplication.

Thus I think it is fine to keep it as it is.

@modelica-trac-importer
Copy link
Author

Comment by hansolsson on 7 Jan 2013 09:33 UTC
Or in summary: safety seems more important than simplifications.

If a tool really needs simplifications it seems it could be done using a tool-specific implementation of ModelicaServices.

Closing since it will not be possible to discuss at a design meeting before the dead-line (since the first one is in march).

@modelica-trac-importer
Copy link
Author

Comment by anonymous on 7 Jan 2013 16:00 UTC
Thank you for your comments and hints. I certainly can understand that you want to move this to the tool vendor side.

Actually, if ModelicaServices is not derived from PartialModelicaServices but rather duplicates its content, this does not change anything for Visualizers.Advanced.Shape as this just means a manually performed inheritance. Infact Visualizers.Advanced.Shape still instantiates components coming from PartialModelicaServices (or its duplicated equivalent) twice (tool-dependently I guess). It only works if Visualizers.Advanced.Shape is no loner derived from PartialModelicaServices but merely from ModelicaServices.

@modelica-trac-importer
Copy link
Author

Comment by hansolsson on 8 Jan 2013 16:02 UTC
Instantiation twice (direct+inherited or inherited+inherited) is not a problem.

The Modelica Specification does (in 3.3 this is in section 7.1) state that an element may be inherited twice without any problem:

"Be exactly identical to any element of the flattened enclosing class with the same name and the same level of protection (public or protected) and same contents. In this case, the first element in order (can be either inherited or local) is kept. It is recommended to give a warning for this case; unless it can be guaranteed that the identical contents will behave in the same way."

So the semantics are given; except for the possible warning - and the tool should be able to avoid that for their own ModelicaServices implementation.

@modelica-trac-importer
Copy link
Author

Comment by anonymous on 9 Jan 2013 10:37 UTC
What does "same content" exactly mean? Same type, same quantity, same dimension, same value?

@modelica-trac-importer
Copy link
Author

Comment by sjoelund.se on 9 Jan 2013 11:04 UTC
Same everything :)

@modelica-trac-importer
Copy link
Author

Comment by hansolsson on 9 Jan 2013 11:07 UTC
Replying to [comment:6 anonymous]:

What does "same content" exactly mean? Same type, same quantity, same dimension, same value?

As Sjoelund wrote:

All of them; "be exactly identical" gives a clear idea of the intent.

However, I believe there have been other trac-tickets on this though, the problem is that two declarations that are character-by-character-identical may behave differently semantically. That's the reason for the recommended warning; but a tool should be able to control that for their own libraries.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Critical/severe issue L: Mechanics.MultiBody Issue addresses Modelica.Mechanics.MultiBody worksforme Clarified issue
Projects
None yet
Development

No branches or pull requests

3 participants