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

simulation hangs if simulated past 363 days #2314

Closed
mwetter opened this issue Dec 18, 2020 · 7 comments · Fixed by #2376
Closed

simulation hangs if simulated past 363 days #2314

mwetter opened this issue Dec 18, 2020 · 7 comments · Fixed by #2376
Assignees
Labels
spawn Development for Spawn of EnergyPlus
Milestone

Comments

@mwetter
Copy link
Member

mwetter commented Dec 18, 2020

Spawn hangs if the stop time is 364 days or 365 days. The following model reproduces it. The first call is for simulation to 363 days, the other two to 364 and 365 days:

simulateModel("Buildings.ThermalZones.EnergyPlus.Validation.ThermalZone.OneZoneOneYear", startTime=31276800, stopTime=31363200, method="Cvode", tolerance=1e-06, resultFile="OneZoneOneYear");
 = true
simulateModel("Buildings.ThermalZones.EnergyPlus.Validation.ThermalZone.OneZoneOneYear", startTime=31276800, stopTime=31449600, method="Cvode", tolerance=1e-06, resultFile="OneZoneOneYear");
 = false
simulateModel("Buildings.ThermalZones.EnergyPlus.Validation.ThermalZone.OneZoneOneYear", startTime=31276800, stopTime=31536000, method="Cvode", tolerance=1e-06, resultFile="OneZoneOneYear");
 = false

The development branch is issue2314_annualSimulation. Also OCT hangs, and various other models show the same behavior.

@mwetter mwetter added the spawn Development for Spawn of EnergyPlus label Dec 18, 2020
mwetter added a commit that referenced this issue Dec 18, 2020
@mwetter
Copy link
Member Author

mwetter commented Dec 18, 2020

@kbenne : Can you please check if E+ shuts down prematurely towards the end of the year? Modelica has no notion of a year, so I suspect that something in E+ may shut down and then block the process.

@kbenne
Copy link
Contributor

kbenne commented Jan 15, 2021

@mwetter I reproduced this in a simple test outside of Modelica. Just setTime on the EnergyPlus FMU past the year and you can reproduce it. EnergyPlus is not shutting down. It simply hangs so that setTime never returns. I'm guessing the simulateModel function is hitting a timeout resulting in the return value "false"

I tracked down the root cause which is that EnergyPlus is hitting the end of the weather file and waiting / looking for more input. The specifically line that it gets hung up on is right here https://github.com/NREL/EnergyPlus/blob/a9778bafea2ee3e92c6671144fa08d1394f11099/src/EnergyPlus/WeatherManager.cc#L8060

If you give EnergyPlus / Spawn a weather file that goes into the next year then this issue will not happen.

EnergyPlus changed behavior around this in version 9.2.0 (and maybe even additional changes since then). Some of this is discussed here https://unmethours.com/question/12917/multiple-year-simulation/ but essentially to the best of my knowledge there is no longer a way to get EnergyPlus to wrap the weather file.

At this point I am not considering this a spawn issue, however at minimum we'll have to document it. If that is not sufficient then. Not sure. I could come up with some kind of workaround, but it might not be elegant.

@mwetter
Copy link
Member Author

mwetter commented Jan 18, 2021

@kbenne : Could E+ be changed so that if a weather file is for a full year (or n-times a year where n is 1, 2, 3, ...) then it assumes the file to be periodic and can simulate any number of years? This is essentially what we do on the Modelica side: At initialization we check if the weather file has a periodicity of a multiple of years, and if so, handle it as a periodic time series (https://github.com/lbl-srg/modelica-buildings/blob/master/Buildings/Resources/C-Sources/getTimeSpan.c#L128 and https://github.com/lbl-srg/modelica-buildings/blob/master/Buildings/BoundaryConditions/WeatherData/BaseClasses/ConvertTime.mo#L22).

Otherwise for Spawn there would need quite some setup just to do an annual simulation. This would also cover a common use case for geothermal simulations where multiple years may need to be simulated to properly account for the ground response.

@kbenne
Copy link
Contributor

kbenne commented Jan 18, 2021

EnergyPlus used to have the option to do the periodic method that you describe, but it seems to have progressively migrated to a more literal approach to date/time. I'll do a bit more research and inquiry with the EnergyPlus developers, but I do not think the periodic option is still there. Let's hold on any decision until I'm 100% certain I have the facts right.

@mwetter mwetter added this to the Release 8.0 milestone Jan 19, 2021
kbenne added a commit to NREL/Spawn that referenced this issue Feb 17, 2021
Previously the particular version of EnergyPlus tied to Spawn would hang
upon reaching the end of the weather file. Now the simulation will wrap
around to the beginning of the weather file as expected. The changes to
Spawn consist of updates to account for EnergyPlus chnages related to
global state refactor. There are functional changes to the Spawn
software.

Tests have been added to demonstrate multi-year tests via the EnergyPlus
FMI.

ref lbl-srg/modelica-buildings#2314
@kbenne
Copy link
Contributor

kbenne commented Feb 17, 2021

@mwetter we should be all set for multi year simulations now. If you read the commit comment you'll see the change amounts to an EnergyPlus update. This commit of Spawn is now current with EnergyPlus develop code. We might have to update some idf files in the MBL but I'm not sure.

While on the topic of EnergyPlus releases. I think moving forward it will be important to develop a policy for coordinating Spawn releases with EnergyPlus releases, just as OpenStudio does already. EnergyPlus is nearing the spring release, and I'm not sure about your exact date for the MBL release, but it might make sense to coordinate so that Spawn matches the release build of EnergyPlus.

@mwetter
Copy link
Member Author

mwetter commented Feb 18, 2021

@kbenne : Thanks, can you please check why https://gitlab.com/kylebenne/spawn/-/pipelines/257812971 failed.

In the future we should think about how to best coordinate once the development is more stable. We also have other dependencies with Modelica tool releases that we usually used to coordinate the semi-annual Buildings release. Another option would be to have a backwards compatible E+ branch. The problem I see with jumping versions prior to E+ releases is that we won't have E+ documentation available to point users to. I am not sure if the E+ master is undergoing the same testings as the releases. If not it may pose a risk that bugs are sneaked into the Buildings master.

For this release would it be a big lift to integrate it on the current E+ version? Otherwise we should consider further delaying the official release as I don't feel comfortable releasing Spawn without having a place to point users for the EnergyPlus idf documentation, unless the upcoming E+ release is backwards compatible.

@kbenne
Copy link
Contributor

kbenne commented Feb 18, 2021

@kbenne : Thanks, can you please check why https://gitlab.com/kylebenne/spawn/-/pipelines/257812971 failed.]

Yes I'm fixing it. The CI virtual machine has run out of disk space.

In the future we should think about how to best coordinate once the development is more stable. We also have other dependencies with Modelica tool releases that we usually used to coordinate the semi-annual Buildings release. Another option would be to have a backwards compatible E+ branch. The problem I see with jumping versions prior to E+ releases is that we won't have E+ documentation available to point users to. I am not sure if the E+ master is undergoing the same testings as the releases. If not it may pose a risk that bugs are sneaked into the Buildings master.

For this release would it be a big lift to integrate it on the current E+ version? Otherwise we should consider further delaying the official release as I don't feel comfortable releasing Spawn without having a place to point users for the EnergyPlus idf documentation, unless the upcoming E+ release is backwards compatible.

I think you mean integrate Spawn on the official EnergyPlus 9.4.0 release. Yes that is significantly difficult. There has been active refactoring on EnergyPlus to enable spawn integrations as well as just a bunch of global state refactoring that Spawn has had to adapt to and undoing it would be involved. I agree you want to release on a specific stable version of EnergyPlus, but I think that would be the upcoming EnergyPlus 9.5.0.

mwetter added a commit that referenced this issue Mar 2, 2021
* Added test case for #2314

* Updated install script for #2223

* Changed code for new json structure of output variables

* Copied file from E+ v9.5.0-IOFreeze

Copied file and added Output:EnergyManagementSystem section

* Updated controlType to 'Electricity Rate'

* Updated documentation for actuators

* Added new binaries. These are for E+ 9.5.0

* Copied file from E+ v9.5.0-IOFreeze

* Removed outputs to avoid E+ warning

* Added text because table is for EnergyPlus 9.4 and not 9.5

* Exclude Spawn E+ model for csv generation

This is because they used E+ 9.5 which has not yet been released
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
spawn Development for Spawn of EnergyPlus
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants