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.

Sign up for GitHub

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

Jump to bottom

Issue1785 modelica conf tutorial #1788

Merged
merged 51 commits into from
Sep 23, 2023
Merged

Issue1785 modelica conf tutorial #1788

merged 51 commits into from
Sep 23, 2023

Conversation

mwetter
Copy link
Contributor

@mwetter mwetter commented Sep 11, 2023

This closes #1785

It adds the tutorial for the Modelica Conference that @jelgerjansen contributed to IBPSA.Examples.Tutorials.

Moving IBPSA.Fluid.Examples.SimpleHouse will be handled through a separate issue, see #1785 (comment) for the rationale.

jelgerjansen and others added 21 commits September 6, 2023 08:58
@jelgerjansen
First version of the intermediate results of the SimpleHouse exercise…
53f86ac 
…, developed using Dymola
@jelgerjansen
Include diagram and experiment annotations in all models since this i…
0cc1328 
…nformation is not included in the extension within OpenModelica
@jelgerjansen
Slightly replace a connection line
6de6c0d
@jelgerjansen
Update documentation of KU Leuven Modelica crash course to be used in…
926f5f6 
… the IBPSA workshop at the Modelica Conference. Major changes:

- Update all IDEAS models to IBPSA models
- Update reference result plots (from Dymola-generated to OpenModelica-generated)
- Restructure document such that every exercise appears on 1 single page
- Add some information to the documentation (like nominal mass flow of the mixing volume)
@mwetter
Merge pull request #1783 from jelgerjansen/modelicaConferenceWorkshop
7c39a70 
IBPSA workshop Modelica Conference
@jelgerjansen
Move the SimpleHouse Modelica models to 'Fluid.Examples.Tutorial.Simp…
3962270 
…leHouse'
@jelgerjansen
Move LaTeX files to IBPSA/Resources/Documentation/Fluid/Examples/Tuto…
34061c2 
…rial/SimpleHouse and clean up directory (put figures in 'img' directory and delete temporary LaTeX files)
@jelgerjansen
Add a short description in the info section of the examples and move …
8250965 
…the Tutorial package to the top of the Examples package
@jelgerjansen
Add regression tests for each model.
8264fef
@jelgerjansen
Add Tolerance to experiment annotation
b55edbd
@mwetter
Made editorial revision
5766402
@jelgerjansen
Add exercise to the info section of the Modelica models.
7665717
@mwetter
Corrected typo
d2a82e0
@mwetter
Added full name (so that renaming works robustly)
5c82475
@mwetter
Increased tolerances to 1E-6, added test case and results
7f93094
@mwetter
Moved tutorial
98c56b0
@mwetter
Corrected hyperlinks
625b981
@mwetter
Deleted old files
e000767
@mwetter
Added IBPSA.Examples to CI tests
5747b73
@mwetter
Corrected html syntax
b63b01d
@mwetter
Corrected annotation
b8951a9
@mwetter
Copy link
Contributor Author

mwetter commented Sep 11, 2023

@FWuellhorst : Would you be able to review this tutorial for the Modelica conference that @jelgerjansen implemented. I did an initial review and some changes, but it is good to be merge in my view. If not, who from Aachen would be able to provide a timely review (and ideally assists during the workshop?

@mwetter mwetter requested a review from FWuellhorst September 11, 2023 23:06
FWuellhorst
FWuellhorst requested changes Sep 12, 2023
View reviewed changes
Copy link
Contributor

@FWuellhorst FWuellhorst left a comment

Choose a reason for hiding this comment

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

The tutorial looks really nice. It all works in OM on Windows.
I have minor style guide comments and had one issue regarding the conditional connect in OM. Maybe you could make it visual by using a switch and a boolean source?

IBPSA/Examples/Tutorial/SimpleHouse/SimpleHouseTemplate.mo Outdated Show resolved Hide resolved
IBPSA/Examples/Tutorial/SimpleHouse/SimpleHouseTemplate.mo Outdated Show resolved Hide resolved
IBPSA/Examples/Tutorial/SimpleHouse/SimpleHouseTemplate.mo Outdated Show resolved Hide resolved
IBPSA/Examples/Tutorial/SimpleHouse/SimpleHouseTemplate.mo Outdated Show resolved Hide resolved
IBPSA/Examples/Tutorial/SimpleHouse/SimpleHouse2.mo Outdated Show resolved Hide resolved
IBPSA/Examples/Tutorial/SimpleHouse/SimpleHouse4.mo Outdated Show resolved Hide resolved
IBPSA/Examples/Tutorial/SimpleHouse/SimpleHouse6.mo Outdated Show resolved Hide resolved
IBPSA/Examples/Tutorial/SimpleHouse/package.mo Show resolved Hide resolved
IBPSA/Examples/package.mo Show resolved Hide resolved
IBPSA/Examples/Tutorial/SimpleHouse/SimpleHouse4.mo Outdated Show resolved Hide resolved
@mwetter
Copy link
Contributor Author

mwetter commented Sep 13, 2023

@jelgerjansen : Would you be able to address the feedback. Also renaming it where we did not use the naming convention would be beneficial so we don't tell users to use some naming convention but then teach them to use some other nomenclature. Unfortunately the SimpleHouse example is quite old and was done before we uniformed the naming convention.

@jelgerjansen
Copy link
Contributor

@FWuellhorst thank you for reviewing this so thoroughly! @mwetter I’ll address the feedback on Friday (once I’m back from the conference)

@jelgerjansen
Rename SimpleHouseTemplate to SimpleHouse0
fa40230
@jelgerjansen
Remove the thermal resistor of the wall from SimpleHouse0 and include…
beb647c 
… it in SimpleHouse1.

Furthermore, the information section of SimpleHouse1 is updated with the modeling information of the thermal resistor.
@jelgerjansen
Rename parameters to comply with the naming convention, fix appearanc…
586d689 
…e of 3kW in documentation
@jelgerjansen
Move window to 'wall' block
32ba036
mwetter
mwetter commented Sep 15, 2023
View reviewed changes
Copy link
Contributor Author

@mwetter mwetter left a comment

Choose a reason for hiding this comment

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

The changes look good to me.

@FWuellhorst FWuellhorst mentioned this pull request Sep 18, 2023
Closed
@FWuellhorst
Copy link
Contributor

@jelgerjansen Thanks for all the fixes, it looks good to me! I pushed a minor connection change, as the red line was not complete in my OMEdit.
@mwetter Please check the discussion on the building envelope. If you agree / disagree, we can merge.

FWuellhorst
FWuellhorst approved these changes Sep 18, 2023
View reviewed changes
Copy link
Contributor

@FWuellhorst FWuellhorst left a comment

Choose a reason for hiding this comment

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

Looks great, thanks!

@mwetter mwetter enabled auto-merge September 20, 2023 02:34
@mwetter mwetter disabled auto-merge September 20, 2023 02:50
@mwetter
Copy link
Contributor Author

mwetter commented Sep 20, 2023

@jelgerjansen : I just noticed an issue in SimpleHouse6: This model uses a hysteresis controller to close the damper. However, the fan is constantly running, which would likely burn out the fan motor.
Could you please change the model and switch the fan off when the damper is closed, and update the plot if needed.
In this model, because the fan used dp as an input, the behavior is not that bad, but we sometimes see users running pumps against closed valves, which can easily cause pressures of a few hundred bars if the pump prescribes the mass flow rate. Hence I suggest we are careful with this configuration.
Note that in IBPSA.Examples.SimpleHouse (on branch issue1791_SimpleHouse) the P controller has yMin=0.25, so there is no issue with a constant dp signal for the fan.

Also, when doing this change, I think it would be easier for this beginner tutorial to use the fan from Movers.Preconfigured. I suggest you make this switch as well.

@FWuellhorst
Copy link
Contributor

OM fixed the issue, we just have to install the nightly build.

@jelgerjansen
Copy link
Contributor

@mwetter I used the models from Movers.Preconfigured for both the circulation pump and the fan, and changed the control of the fan such that it is only active when the damper is open (using the hysteresis output signal). Furthermore, I updated the documentation (including updating the 'required models' section and putting these in alphabetical order).

@mwetter
Copy link
Contributor Author

mwetter commented Sep 22, 2023

@jelgerjansen : The tutorial looks good to me. I formatted the indentation, corrected the html formatting to comply with the coding convention (use <code> rather than <i> for code) and the main change was to conditionally remove the components rather than the connections for the control signal.

@mwetter mwetter enabled auto-merge September 22, 2023 15:12
@mwetter mwetter merged commit 2d4f7c5 into master Sep 23, 2023
3 checks passed
@mwetter mwetter deleted the issue1785_modelicaConfTutorial branch September 23, 2023 08:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jelgerjansen jelgerjansen jelgerjansen left review comments

@FWuellhorst FWuellhorst FWuellhorst approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Create tutorial for use at Modelica Conference
3 participants
@mwetter @jelgerjansen @FWuellhorst