ModelicaStandardLibrary
ModelicaStandardLibrary copied to clipboard
Controlled drives
I'd like to enhance the examples Modelica.Electrical.Machines.Examples.ControlledDCDrives, including a UseresGuide with some explanations. The delays and the timing that have to be taken into account are explained in more detail. For a proper solution, I'd need to merge #3806 first. I.e. after that, I'll replace the instance of the enhanced block "Mean" by the block from master.
OK, the structure looks good so far. Let's wait for input from #3814 to fully approve this PR.
I have one more general comment of these types of examples that depend on new components that are then placed into a subpackage (e.g., Utilities
). The problem here is that we in future say, well things inside Examples
should not be extended from so we do not have to care about backward compatibility. So this leads to users getting broken models when making use of Utilities/*
models. The question here is, shouldn't be the purpose of examples be to demonstrate what you can do with the respective library without having to come up with additional components (which then get parked away inUtilities
or similar). And if an example is depending on such an additional component then does this not signal that the actually library is still missing something.
Personally I can think of many sophisticated examples that are based on MSL but are lacking one or two special components. But that should not be a reason to put those examples into the MSL. Rather make the MSL examples show what you can do with the available standard components. And if it turns out that there is such an important example that is missing some key-component, then maybe this key component should be added to the library the example is showing off. And if it turns out that it is not of such a general importance then maybe the example should be changed to not depend on such additional components.
@dietmarw would you please have a look at the fix provided?
@TManikantan Before sending these reminders it would help if you would read what was said in the ticket. Also it might perhaps be a better idea that you make yourself familiar with the people involved in the project.
We concur with @dietmarw response. Use of models inside the Utilities sub package is not an ideal way of showcasing the capabilities of MSL models. However, this is a recurring issue in multiple subpackages – listed in option 2. We suggest two options: 1.Given that this ticket deals with examples in ControlledDCDrives, and they seem to be working fine, let us close this ticket after #3814 is addressed. 2.There are multiple instances in other sub-packages too. Classes which are not backward compatible should be moved out of Utilities into respective packages if they are general standard component. However, these should be addressed carefully as a separate issue per package.
Few Examples which depend on new components placed in utilities :
-
Modelica.Blocks.Examples.Noise.Densities
-
Modelica.Clocked.Examples.Systems.ControlledMixingUnit
-
Modelica.StateGraph.Examples.ControlledTanks
-
Modelica.Magnetic.FluxTubes.Examples.MovingCoilActuator.ForceCurrentBehaviour
-
Modelica.Mechanics.MultiBody.Examples.Loops.EngineV6
Regarding the components in this Utilities package, here are some of our suggestions:
-
Modelica.Electrical.Machines.Examples.ControlledDCDrives.Utilities.LimitedPI
looks similar to the componentModelica.Blocks.Continuous.LimPID
. Suggestion would be to ReuseModelica.Blocks.Continuous.LimPID
and if not possible create a new component for controllers than working on top of a controller strategy. -
Specific controllers i.e current, speed and position controllers have the potential to be reused and Suggestion would be to add these controllers in another dedicated location inside DC machines in
Modelica.Electrical.Machines.BasicMachines.DCMachines
or at a similar hierarchy.
https://github.com/modelica/ModelicaStandardLibrary/pull/3807#issuecomment-1547850161
Since these are nice running examples, I would also accept them as they are now (suggestion (1)). Afterwards, one should open a new ticket, to move sub-models below Utilities that are also useful in another context (LimitedPI, Battery,. ..) into a better place (maybe with improved documentation) and then modify these examples correspondingly.
Since this PR is open for a long time, some aspects have changed and I'd prefer to check and modify the examples beforehand.
Since these are nice running examples, I would also accept them as they are now (suggestion (1)). Afterwards, one should open a new ticket, to move sub-models below Utilities that are also useful in another context (LimitedPI, Battery,. ..) into a better place (maybe with improved documentation) and then modify these examples correspondingly.
I'm sorry @MartinOtter I strongly disagree here. Exactly this kind of routine has led to an MSL that currently is on the brink of staying maintainable (given our lack of resources and engagement). So instead of adding new features that are not quite rightly implemented and require more work later on (and also much more involved because of conversion scripts, when, by whom) I rather suggest we do it right the first time. And if it is too much work now, then maybe not add this to the MSL then since it will definitely end up becoming more clean-up work later on.
Personally I can think of many sophisticated examples that are based on MSL but are lacking one or two special components. But that should not be a reason to put those examples into the MSL. Rather make the MSL examples show what you can do with the available standard components. And if it turns out that there is such an important example that is missing some key-component, then maybe this key component should be added to the library the example is showing off. And if it turns out that it is not of such a general importance then maybe the example should be changed to not depend on such additional components.
I respectfully disagree. It is (quite often) the case that one can re-use 70-90% of components provided by the MSL (or any other library) and requires to develop a few models on his/her own. This is a very common pattern and I can't see anything wrong in doing that in Examples. In fact, I find it of educational value. The components put there may not be general enough to deserve a place in the library, this is also perfectly legitimate.
I understand we have a potential problem with dependencies because people could start using the models found in Examples and then we create further backward compatibility issues. This could be resolved by placing Examples in a separate package, but that's not really practical. I think we should resolve this issue simply by having dedicated dependency analysis tools that simply skip packages named Examples (or possibly marked by a suitable ExamplePackage annotation).
Restricting the example to toy models that are exclusively built with basic components from the MSL seems an unnecessary restriction to me.
IMHO if this is the only reason why this PR is still not merged after 2 1/2 years, I would merge it right away. This is the current situation
- one library officer is the proponent
- the other library officer is OK with the PR
- the current MAP-Lib leader is OK with the PR if there are no other issues than the examples
- the previous deputy MAP-Lib leader is OK with the PR
- the former MAP-Lib leader objects to having new models in Examples
To me the situation is pretty clear. Any comments?