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

Made engine designs consistent w.r.t. the whole design scenario #3133

Merged
merged 75 commits into from
Oct 9, 2019

Conversation

christoff-buerger
Copy link
Member

@christoff-buerger christoff-buerger commented Oct 4, 2019

This pull request fixes the inconsistencies between the Engine1a, Engine1b and Engine1b_analytic models of Mechanics.MultiBody.Examples.Loops. For details about these, please cf. Section 4.2 of Modelica language extensions for practical non-monotonic modelling: on the need for selective model extension.

The pull request fixes #3132.

The changes are:

  1. Consistent stop times:
    • Engine1b, Engine1b_analytic: stop time set from 0.5 to 5 (the time of Engine1a)
  2. Consistent inertias:
    • Engine1bBase (=> Engine1b, Engine1b_analytic): set inertia(J = 1 "kg m^2") (the inertia of Engine1a)
  3. Consistent FixedTranslation of cycle position:
    • Engine1bBase (=> Engine1b, Engine1b_analytic): set cylPosition(r = {0.15,0.45,0}) (the cycle position of Engine1a)
  4. Consistent piston <-> cylinder connections:
    • Engine1b: connected pistion.a with cyclinder.b instead of rod3.a (the connection of Engine1a)
  5. Consistent bearings:
    • Engine1b:
      • switched bearings b1 and b2 (according to Engine1a)
      • turned rod1 and connectingRod respectively
      • set rod1.r and connectingRod.r to {0,-0.2,0} (according to Engine1a)
  6. Improved, consistent graphical layout

bernhard-thiele and others added 30 commits May 22, 2019 17:16
Goal: Integrate the Modelica_Synchronous library into the MSL 4.0.0 under
the name Modelica.Clocked.

Reference to the code which is the starting point of this integration
task: modelica/Modelica_Synchronous@0416c85
Manually fix remaining text occurences related to the switch from "Modelica_Synchronous" to Modelica.Clocked.
The models in WorkInProgress.mo are experimental and nobody
worked at them for a long time. Should be moved out of the
main repository (may preserve the package in a feature branch
in my fork of the main repo).
Using the suffix _der to denote a derivative is more idiomatic Modelica code
Removed as requested in review, related to discussion in modelica#2494.
Replaced assert(not trigger_interval==0, "...") by assert(trigger_interval > 0 or trigger_interval < 0, "...")
Copy link
Member

@beutlich beutlich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice.

…e now diagramatically represented, improved diagram layout, improved documentation, eliminated rotational clock duplicate and use of engine synchronization instead as single source of sampling).
…e now diagramatically represented, improved diagram layout, improved documentation, eliminated rotational clock duplicate and use of engine synchronization instead as single source of sampling).
@dietmarw
Copy link
Member

dietmarw commented Oct 8, 2019

Please rebase on current master.

@beutlich
Copy link
Member

beutlich commented Oct 8, 2019

@christoff-buerger Can we please have two separate fixes here:

  1. One, targeting the existing models of MultiBody which I already reviewed.
  2. Another one, targeting Modelica.Clocked which you can directly commit to the corresponding feature branch (since Modelica.Clocked is new in MSL anyway).

…lings are now diagramatically represented, improved diagram layout, improved documentation, eliminated rotational clock duplicate and use of engine synchronization instead as single source of sampling)."

This reverts commit 9dc348d.

As requested in modelica#3132 by @beutlich.
…lings are now diagramatically represented, improved diagram layout, improved documentation, eliminated rotational clock duplicate and use of engine synchronization instead as single source of sampling)."

This reverts commit ad35571.

As requested in modelica#3132 by @beutlich.
@christoff-buerger
Copy link
Member Author

@christoff-buerger Can we please have two separate fixes here:

  • One, targeting the existing models of MultiBody which I already reviewed.
  • Another one, targeting Modelica.Clocked which you can directly commit to the corresponding feature branch (since Modelica.Clocked is new in MSL anyway).

@beutlich: I am sorry for the chaos; I merged the synchronous stuff in by accident due to missing rights. It should be alright now again (I reverted the problematic commit for the pull-request).

@beutlich
Copy link
Member

beutlich commented Oct 9, 2019

I am sorry for the chaos

I'll do a squash-merge later such that the back-and-forth commits will be removed from history,

Copy link
Contributor

@tobolar tobolar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, some consistency requirements are really tough, e.g.

  1. Consistent piston <-> cylinder connections:
    Engine1b: connected pistion.a with cyclinder.b instead of rod3.a (the connection of Engine1a)

But fine to me. Now I even changed Engine1a to extend from Engine1bBase as well.

@tobolar tobolar changed the title #3132: Made engine designs consistent w.r.t. the whole design scenario. Made engine designs consistent w.r.t. the whole design scenario Oct 9, 2019
@beutlich beutlich removed the request for review from MartinOtter October 9, 2019 15:11
@beutlich beutlich merged commit e6c7cca into modelica:master Oct 9, 2019
@beutlich
Copy link
Member

beutlich commented Oct 9, 2019

For the future, it is safer if you do not create PRs from master. You should use dedicated branches instead.

@christoff-buerger
Copy link
Member Author

christoff-buerger commented Oct 10, 2019

Now I even changed Engine1a to extend from Engine1bBase as well.

@tobolar: I do not understand how that can be done? Engine1a is a model without the gas force of the cylinder; the intention of Engine1bBase is to introduce the gas force such that Engine1b and Engine1b_analytic consider the gas-force. The whole catch of this inheritance hierarchy is that it is cross-cutting and cannot be modeled as a straight forward inheritance.

I think your commit fa75454 is wrong and should be undone!

@christoff-buerger
Copy link
Member Author

christoff-buerger commented Oct 10, 2019

I checked now the version with the last changes of @tobolar, particular fa75454.

We should revisit this. The now common base model Engine1bBase has as description "Model of one cylinder engine with gas force", but the gas-force has been removed in it. Instead, Engine1b and Engine1b_analytic individually reintroduce it.

I see two approaches to fix this:

  1. Undo fa75454

or the following steps

  1. Change the description of Engine1bBase to "Common model of one cylinder engine without gas force."
  2. Introduce another new base-class, Engine1bBase_GasForce, which also includes the gas-force and inherits from Engine1bBase; its description would be "Common model of one cylinder engine with gas force."
  3. Let Engine1b and Engine1b_analytic inherit from Engine1bBase_GasForce and Engine1a inherit from Engine1bBase.

@beutlich: How do you like us to proceed? New branch and pull request?

@beutlich
Copy link
Member

New branch and pull request?

Exactly. Thank you.

christoff-buerger added a commit to christoff-buerger/ModelicaStandardLibrary that referenced this pull request Oct 10, 2019
christoff-buerger added a commit to christoff-buerger/ModelicaStandardLibrary that referenced this pull request Oct 11, 2019
christoff-buerger added a commit to christoff-buerger/ModelicaStandardLibrary that referenced this pull request Oct 30, 2019
beutlich pushed a commit to beutlich/ModelicaStandardLibrary that referenced this pull request Nov 29, 2019
beutlich pushed a commit to christoff-buerger/ModelicaStandardLibrary that referenced this pull request Dec 5, 2019
beutlich added a commit that referenced this pull request Dec 5, 2019
Fix for last-minute errors of #3133 pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
example Issue only addresses example(s) L: Mechanics.MultiBody Issue addresses Modelica.Mechanics.MultiBody
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inconsistencies between engines of Mechanics.MultiBody.Examples.Loops
5 participants