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
466 stars 166 forks source link

New quasi-static flux tubes library to be integrated in MSL #1515

Closed modelica-trac-importer closed 6 years ago

modelica-trac-importer commented 7 years ago

Reported by ahaumer on 14 Jun 2014 11:19 UTC Christian Kral developed a quasi static version of (parts of) the flx tubes library, i.e. assuming sinusoidal waveforms with respect to time. The actual version of this library is located at ​ https://svn.modelica.org/projects/Modelica_ElectricalSystems/Modelica_QuasiStatic_FluxTubes/trunk. This tickes is intended to summarize issues required to integrate the quasistatic flux tubes library into the MSL.


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

modelica-trac-importer commented 7 years ago

Comment by sjoelund.se on 14 Jun 2014 13:06 UTC I believe the library needs more examples to test the models in the library:

Used by examples:

Unused:

modelica-trac-importer commented 7 years ago

Modified by dietmarw on 20 Jun 2014 13:14 UTC

modelica-trac-importer commented 7 years ago

Comment by christiankral on 6 Dec 2014 14:21 UTC The leakage flux components in Shapes.Leakage are included in the QuasiStatic.FluxTubes library for compatibility reasons with Magnetic.FluxTubes. Since there are currently no examples included in Magnetic.FluxTubes, there are also no examples included in QuasiStatic.FluxTubes -- except for Examples.Leakage.CylinderLeakage and Examples.Leakage.GeneralLeakage.

However, for testing purposes I included an example in MoveToModelicaTest.NoPhysicalTestLeakage. These examples shall then be included in ModelicaTest after moving the package to MSL. The example is not based on a physical model of magnetic circuit; instead it represents a test case for those models which do not have any example in QuasiStatic.FluxTubes. This is kind of a workaround until test examples are provided in Magnetic.FluxTubes, so that they can also be applied to QuasiStatic.FluxTubes.

Consequently, for each component of QuasiStatic.FluxTubes there is an example included in r7956 either in the package itself or in (MoveTo)ModelicaTest.

modelica-trac-importer commented 7 years ago

Comment by christiankral on 5 Aug 2015 19:39 UTC All actual issues are fixed with this package. However, since it was not yet decided to include this package in the MSL, I am shifting the milestone.

christiankral commented 7 years ago

Moved repository from https://svn.modelica.org/projects/Modelica_ElectricalSystems/Modelica_QuasiStatic_FluxTubes to https://github.com/christiankral/Modelica_QuasiStatic_FluxTubes

Updated copyright year and dependency on Complex 3.2.2 (instead of 3.2.1).

christiankral commented 7 years ago

How is it planned to come up with a decision on whether the library shall be included or not?

I propose to make a vote on the inclusion.

AHaumer commented 6 years ago

Christian, I suggest you create a pull request. BTW, I'm in favour.

dietmarw commented 6 years ago

The version as proposed in #2538 and unfortunately already merged has some signifcant issues. You really should not merge such new additions before leaving some acceptable time for people to look at it.

One of the issues is that QuasiStatic.FluxTubes.Basic contains a lot of components that are duplicates of the already existing FluxTubes.Basic components. This should really be avoided. Als the copyright statement of such duplicates is then questionable.

MSL really should not ship lots of duplicate models unless really necessary.

dietmarw commented 6 years ago

There are actually many other duplicates not just the Basic content unfortunately.

beutlich commented 6 years ago

Modelica.Magnetic.QuasiStatic.FluxTubes.Interfaces.AbsoluteSensor, Modelica.Magnetic.QuasiStatic.FluxTubes.Interfaces.RelativeSensor and Modelica.Magnetic.QuasiStatic.FluxTubes.Interfaces.Source do not match the new Icon convention w.r.t. the component name.

christiankral commented 6 years ago

One of the issues is that QuasiStatic.FluxTubes.Basic contains a lot of components that are duplicates of the already existing FluxTubes.Basic components. This should really be avoided. Als the copyright statement of such duplicates is then questionable.

@dietmarw If you argue that the models of Magnetic.QuasiStatic.FluxTubes are duplicates of Magnetic.FluxTubes then you also have to argue that Electrical.QuasiStationary.SinglePhase contains duplicates of Electrical.Analog and Electrical.QuasiStationary.MultiPhase contains duplicates of Electrical.MultiPhase. But this is certainly not the case, as the "duplicate" models have different connectors and thus they not duplicates. The difference is:

christiankral commented 6 years ago

Modelica.Magnetic.QuasiStatic.FluxTubes.Interfaces.AbsoluteSensor, Modelica.Magnetic.QuasiStatic.FluxTubes.Interfaces.RelativeSensor and Modelica.Magnetic.QuasiStatic.FluxTubes.Interfaces.Source do not match the new Icon convention w.r.t. the component name.

@dietmarw What "new Icon convention" do you refer to? Please direct me to it.

beutlich commented 6 years ago

@dietmarw What "new Icon convention" do you refer to? Please direct me to it.

See https://github.com/modelica/ModelicaStandardLibrary/blob/4f4b99661dee022f506916325d72486413fcb119/Modelica/package.mo#L1860-L1894

christiankral commented 6 years ago

Sorry for confusing the author: @beutlich What "new Icon convention" do you refer to? Please direct me to it.

beutlich commented 6 years ago

See my link above. The component %name should be 40 units in height and 300 units in width. This does not hold for many newly introduced classes.

christiankral commented 6 years ago

Ohhh... I see. The location and size of %name is not correct. I also found some other components not matching the convention. I will fix it.

dietmarw commented 6 years ago

@christiankral

@dietmarw If you argue that the models of Magnetic.QuasiStatic.FluxTubes are duplicates of Magnetic.FluxTubes then you also have to argue that Electrical.QuasiStationary.SinglePhase contains duplicates of Electrical.Analog and Electrical.QuasiStationary.MultiPhase contains duplicates of Electrical.MultiPhase. But this is certainly not the case, as the "duplicate" models have different connectors and thus they not duplicates.

The problem was that I did only see the identical code in the components. E.g., *.FluxTubes.Basic.ConstantReluctance:

model ConstantReluctance "Constant reluctance"
  extends FluxTubes.Interfaces.PartialTwoPorts;
  parameter SI.Reluctance R_m=1 "Magnetic reluctance";
equation 
  V_m = Phi*R_m;
  annotation (...);
end ConstantReluctance;

without realising that relative extends is actually pointing to two different interface types. The curse of relative class definitions ...

So sorry about that. Still one might wonder if there might be a more elegant way of reuse the basic components with different interfaces.

beutlich commented 6 years ago

There are some concerns about missing/insufficient documentation. See https://gistpreview.github.io/?c6e6420f5fc7509e1931f0695b06b6e9

christiankral commented 6 years ago

I will fix this issues.

christiankral commented 6 years ago

@beutlich We should possible investigate other subpackages of the MSL in a similar way as shown in https://gistpreview.github.io/?c6e6420f5fc7509e1931f0695b06b6e9. I suspect, we have couple more issues...

christiankral commented 6 years ago

@beutlich Something went wrong, unfortunately, with the merge in 81b04aed4df1b70a6545c262d31e407fdc61cf59

christiankral commented 6 years ago

I am a sorry, I could not figure out from the listed merge conflicts that I am undoing some of your changes...

christiankral commented 6 years ago

The unwanted changes happend in e6486b7c20eda9069d73b57b8f603a4b2c0ab66e, not 81b04ae

christiankral commented 6 years ago

Thanks.

beutlich commented 6 years ago

@beutlich We should possible investigate other subpackages of the MSL in a similar way as shown in https://gistpreview.github.io/?c6e6420f5fc7509e1931f0695b06b6e9. I suspect, we have couple more issues...

Yes, only checked MultiBody (as part of https://github.com/modelica/ModelicaStandardLibrary/issues/285#issuecomment-358330204) and this new package so far.

beutlich commented 6 years ago

There are some concerns about missing/insufficient documentation. See https://gistpreview.github.io/?c6e6420f5fc7509e1931f0695b06b6e9

Looks better now, but still one missing documentation.

christiankral commented 6 years ago

@beutlich Fixed all missing and possibly insufficient documentation.