modelica / ModelicaStandardLibrary

Free (standard conforming) library to model mechanical (1D/3D), electrical (analog, digital, machines), magnetic, thermal, fluid, control systems and hierarchical state machines. Also numerical functions and functions for strings, files and streams are included.
https://doc.modelica.org
BSD 3-Clause "New" or "Revised" License
472 stars 168 forks source link

Proper location of adaptors #2629

Open tobolar opened 6 years ago

tobolar commented 6 years ago

As partial adaptors FlowToPotentialAdaptor and PotentialToFlowAdaptorare both placed in Blocks.Interfaces.Adaptors, I would expect that all adaptors which extend them are placed in corresponding packages as well, i.e. in Interfaces. But they are all placed in Components of particular domains instead.

This probably originates from location of "old" adaptors in Mechanics.Rotational.Components but should be fixed.

As another example, FlangeWithBearingAdaptor is properly placed in Mechanics.MultiBody.Interfaces.

In contrast, Mechanics.Rotational.Components.AngleToTorqueAdaptor and Mechanics.Rotational.Components.TorqueToAngleAdaptor should of course be moved in the next major version at the earliest.

tobolar commented 6 years ago

I could prepare a PR if there is a consent on this issue.

beutlich commented 6 years ago

I had the same thoughts when testing the new adaptors. But given that the old rotational adaptors are located in Components (and not in Interfaces), I stopped worrying as it is at least consistent with all the non-partial adaptors always located in Components.

Thus, I'd keep the adaptors as is for v3.2.3.

tobolar commented 6 years ago

I want to mention this before releasing v3.2.3. The location is consistent now but is it correct as well?

So we can

  1. preservate current implementation as the proper one,
  2. preservate current implementation keeping in mind this has to be fixed in the next major, or
  3. fix it right now and keep in mind to convert old adaptors' location in the next major.

And there is also this opposite case of Mechanics.MultiBody.Interfaces.FlangeWithBearingAdaptor.

We can leave this issue open for a while and keep discussion running.

svorkoetter commented 6 years ago

I don't think we can move them for 3.2.3 as this would break backward compatibility.

tobolar commented 6 years ago

Both partial adaptors and extended general adaptors are newly introduced in v3.2.3. So it should be no problem to fix it.

More important for me is whether the current location of general adaptors in Components is done on purpose or by mistake.

AHaumer commented 6 years ago

Well, the "old" adaptors were located in components, so the "new" ones are located here, too. If I were a user, I would search for the adaptors in components, not in interfaces. The PartialAdaptors are loacted in Interfaces, just like many other Partials.

dietmarw commented 6 years ago

The adaptors are basically interfaces to a different sub-library. It is very important to differentiate between components of a sub-library and interfaces to other sub-libraries. I've already presented the importance and dangers of having deeply nested components that use classes from other domain sub-libraries (84th Design Meeting).

So the user needs to be educated that whenever you want to interface with something, it is under "Interfaces" you will find this.

tobolar commented 6 years ago

There is probably far more stuff concerned within the MSL, since there is e.g. PlugToPin_p located in Electrical.QuasiStationary.MultiPhase.Basic but 'FlangeWithBearingAdaptor' in Mechanics.MultiBody.Interfaces (I know I repeat myself). So probably we have to collect all the stuff first and to classify it somehow in order to decide what is the proper location.