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

Features/storage validation #162

Merged
merged 18 commits into from
Sep 23, 2020
Merged

Features/storage validation #162

merged 18 commits into from
Sep 23, 2020

Conversation

jakob-wo
Copy link
Contributor

This PR adds a validation of the stratified thermal storage model to the documentation.
The code behind it (creates the figure) is added to the example section.

@jakob-wo jakob-wo self-assigned this Sep 15, 2020
@lgtm-com
Copy link

lgtm-com bot commented Sep 15, 2020

This pull request introduces 1 alert when merging abda094 into a70c542 - view on LGTM.com

new alerts:

  • 1 for Module is imported with 'import' and 'import from'

Jakob Wolf added 2 commits September 16, 2020 10:06
- Avoid to import same package with 'import' and 'import from'
- Make code confirm with LGTM recommendation
@jakob-wo
Copy link
Contributor Author

@c-moeller please have a look at the documentation and let me know if you understand how the validation was conducted from what I wrote so far.

Copy link
Member

@c-moeller c-moeller left a comment

Choose a reason for hiding this comment

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

@jakob-wo Thank you for the validation! It reads good and understandable. I've just made some small remarks.

Could you give a final statement about the storage model below the figure? Does it calculate losses in an appropriate way for energy system analysis? And why are the measurement data fluctuating that much? Measuring inaccuracy?

And one final question: You did the validation for a quite high storage level. Maybe you can add a line, why this is sufficient. Or should we contrast a validation with lower storage levels?

docs/validation_stratified_thermal_storage.rst Outdated Show resolved Hide resolved
docs/validation_stratified_thermal_storage.rst Outdated Show resolved Hide resolved
docs/validation_stratified_thermal_storage.rst Outdated Show resolved Hide resolved
docs/validation_stratified_thermal_storage.rst Outdated Show resolved Hide resolved
docs/validation_stratified_thermal_storage.rst Outdated Show resolved Hide resolved
docs/validation_stratified_thermal_storage.rst Outdated Show resolved Hide resolved
docs/validation_stratified_thermal_storage.rst Outdated Show resolved Hide resolved
docs/validation_stratified_thermal_storage.rst Outdated Show resolved Hide resolved
docs/validation_stratified_thermal_storage.rst Outdated Show resolved Hide resolved
jakob-wo and others added 11 commits September 22, 2020 09:09
Co-authored-by: Caroline Möller <caroline.moeller@rl-institut.de>
Co-authored-by: Caroline Möller <caroline.moeller@rl-institut.de>
Co-authored-by: Caroline Möller <caroline.moeller@rl-institut.de>
Co-authored-by: Caroline Möller <caroline.moeller@rl-institut.de>
Co-authored-by: Caroline Möller <caroline.moeller@rl-institut.de>
Co-authored-by: Caroline Möller <caroline.moeller@rl-institut.de>
Co-authored-by: Caroline Möller <caroline.moeller@rl-institut.de>
Co-authored-by: Caroline Möller <caroline.moeller@rl-institut.de>
@jakob-wo
Copy link
Contributor Author

@c-moeller, thank you for the review!

I added the missing text in the results section and considered your suggestions in the changes.
Please, have a look at the new text and let me know if it answers the questions sufficiently.

Copy link
Member

@c-moeller c-moeller left a comment

Choose a reason for hiding this comment

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

Looks good. Thank you.
I've just made some small change in the table. Sorry, maybe it was not clear what I meant.

docs/validation_stratified_thermal_storage.rst Outdated Show resolved Hide resolved
jakob-wo and others added 2 commits September 23, 2020 18:10
Co-authored-by: Caroline Möller <caroline.moeller@rl-institut.de>
Co-authored-by: Caroline Möller <caroline.moeller@rl-institut.de>
@jakob-wo
Copy link
Contributor Author

@c-moeller: Great! I am going to merge it then. Thank you.

@jakob-wo jakob-wo merged commit 8100bbe into dev Sep 23, 2020
@jakob-wo jakob-wo deleted the features/storage_validation branch September 23, 2020 17:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants