lbl-srg / modelica-buildings

Modelica Buildings library
257 stars 159 forks source link

Update Spawn to EnergyPlus 24.2.0 #3911

Closed kbenne closed 1 week ago

Mov0 commented 3 months ago

May I ask how this is moving along? An update soon to a more recent EnergyPlus version would be much needed here, as the IDFVersion Updater does not allow to downgrade.

mwetter commented 3 months ago

I expect to issue a release in the next couple of months.

kbenne commented 3 months ago

I posted a early buiild for Linux that supports EnergyPlus 24.1.0 here

I'm moving forward on a branch that folds in the new autosizing variables. My preference is to wrap this up and then do an update that includes the EnergyPlus update and the autosizing update, but if you want to roll out the EnergyPlus update by itself we could do that.

Mov0 commented 3 months ago

I am currently in a situation where it was not clear from the beginning that I would be using SOEP and I was not aware that it was based on EnergyPlus 9.6.0. Therefore, I spent two months creating eight building models in OpenStudio-1.7.1/EnergyPlus-23.2.0 that I would have to manually downgrade to EnergyPlus 9.6.0 in order to use them with SOEP. If the EnergyPlus update could be released on its own, I would be very grateful, but if it disrupts your intended workflow too much, I understand.

kbenne commented 3 months ago

Can you use Linux or do you need a windows build?

Mov0 commented 3 months ago

I am unfortunately dependent on Windows because our Dymola licenses are used on Windows PCs.

mwetter commented 2 months ago

@kbenne : I tested an intermediate release on branch issue3911_EP_24_1_0, e.g., hash 2b19281ab8 that uses https://spawn.s3.amazonaws.com/custom/Spawn-light-0.6.0-69002307aa-Linux.tar.gz

On Dymola, this fails with

Process Simulating: Initialization
 stopped without any error indication.

--idd: File does not exist: /home/mwetter/proj/ldrd/bie/modeling/github/lbl-srg/modelica-buildings/Buildings/Energy+.idd
Run with --help for more information.
terminate called after throwing an instance of 'CLI::ValidationError'
  what():  --idd: File does not exist: /home/mwetter/proj/ldrd/bie/modeling/github/lbl-srg/modelica-buildings/Buildings/Energy+.idd
Aborted (core dumped)

Did anything regarding handling the idd file change? The idd file is here after the crash:

$ find . -name Energy+.idd
./Resources/bin/Spawn-light-0.6.0-69002307aa/linux64/etc/Energy+.idd
./spawn-Unconditioned/resources/Energy+.idd

It looks like E+ expects it in the working directory rather than picking the one from ./spawn-Unconditioned/resources/Energy+.idd that is being created. Also the core dumped is unexpected.

mwetter commented 2 months ago

@kbenne : Kyle, did you isolate what causes this change in idd handling, and is the path forward to recreate the old behavior (as with the current Spawn coupling).

kbenne commented 1 month ago

@mwetter as we discussed via email, I updated the IDF files from v24.1.0 to v24.2.0 here. Can you create the remaining directories and files to support 24.2.0?

There is one more thing, which is that as of v24.2.0 EnergyPlus has dropped support for Ubuntu 20.04 and now the minimum version is Ubuntu 22.04. Should we follow?

kbenne commented 1 month ago

@kbenne : Kyle, did you isolate what causes this change in idd handling, and is the path forward to recreate the old behavior (as with the current Spawn coupling).

Yes. idd path issues are resolved now.

mwetter commented 1 month ago

@kbenne : Let's stay with Ubuntu 20.04 as later versions cause issues with Dymola. Please let me know when binaries for Spawn are available so I can add the right hash to https://github.com/lbl-srg/modelica-buildings/pull/4018 and test it.

kbenne commented 4 weeks ago

@mwetter These are based on EnergyPlus v24.2.0a. My tests are functional and qualitative. You will have better quantitative tests. We should expect some change in the results, but I will be interested to learn what the magnitude of the difference is.

https://spawn.s3.amazonaws.com/custom/Spawn-light-0.6.0-3794215efe-Linux.tar.gz https://spawn.s3.amazonaws.com/custom/Spawn-light-0.6.0-3794215efe-win64.zip

mwetter commented 3 weeks ago

@kbenne : Thanks for the binaries. I tested them on issue3911_EP_24_2_0_mw but there is an issue when writing to surface temperatures: Running simulateModel("Buildings.ThermalZones.EnergyPlus_24_2_0.Validation.SurfaceComparison.SurfaceComparison", stopTime=2592000, method="Cvode", tolerance=1e-06, resultFile="SurfaceComparison"); creates a dslog.txt file that has lot of entries of the form

600.000 SurfaceComparison.zonSur.building: Warning from EnergyPlus: Data Exchange API: You seem to already have tried to get an Actuator Handle on this one.
600.000 SurfaceComparison.zonSur.building: Warning from EnergyPlus: Occurred for componentType='SURFACE', controlType='SURFACE INSIDE TEMPERATURE', uniqueKey='GARAGE:INTERIOR'.
600.000 SurfaceComparison.zonSur.building: Warning from EnergyPlus: The getActuatorHandle function will still return the handle (= 279) but caller should take note that there is a risk of overwritting.
600.000 SurfaceComparison.zonSur.building: Warning from EnergyPlus: Data Exchange API: You seem to already have tried to get an Actuator Handle on this one.

at every time step, and the simulation takes about 30 seconds for 1 day. (This model should take almost no time to simulate a day.)

Can you please look into this and add a test to write to zoneSurfaces.

kbenne commented 3 weeks ago

Thank you for testing @mwetter. I do have tests on the surface IO, but I didn't notice the perfermance regression. I think I can fix this today.

kbenne commented 3 weeks ago

@mwetter These should fix the performance regression.

https://spawn.s3.us-east-1.amazonaws.com/custom/Spawn-light-0.6.0-db168408b0-Linux.tar.gz https://spawn.s3.us-east-1.amazonaws.com/custom/Spawn-light-0.6.0-db168408b0-win64.zip

mwetter commented 3 weeks ago

@kbenne : The warnings go away, but the reported units are wrong: In 9.6.0 (as on the master branch), I get for this model

Output SurfaceComparison.zonSur.wes.extSurHea.y has in Modelica the unit W.
Output SurfaceComparison.zonSur.eas.extSurHea.y has in Modelica the unit W.
Output SurfaceComparison.refSur.TGarAir.y has in Modelica the unit K.
Output SurfaceComparison.refSur.TIntWalLivSur.y has in Modelica the unit K.
Output SurfaceComparison.refSur.TIntWalGarSur.y has in Modelica the unit K.
Output SurfaceComparison.refSur.TWesWalGarSur.y has in Modelica the unit K.
Output SurfaceComparison.refSur.TEasWalGarSur.y has in Modelica the unit K.
Output SurfaceComparison.zonSur.TGarAir.y has in Modelica the unit K.
Output SurfaceComparison.zonSur.TIntWalLivSur.y has in Modelica the unit K.
Output SurfaceComparison.zonSur.TIntWalGarSur.y has in Modelica the unit K.
Output SurfaceComparison.zonSur.TWesWalGarSur.y has in Modelica the unit K.
Output SurfaceComparison.zonSur.TEasWalGarSur.y has in Modelica the unit K.
Model: Buildings.ThermalZones.EnergyPlus_9_6_0.Validation.SurfaceComparison.SurfaceComparison
Integration started at 0 using integration method:

With today's version, I get

Output SurfaceComparison.zonSur.wes.extSurHea.y has in Modelica the unit 1.
Output SurfaceComparison.zonSur.eas.extSurHea.y has in Modelica the unit 1.
Output SurfaceComparison.refSur.TGarAir.y has in Modelica the unit 1.
Output SurfaceComparison.refSur.TIntWalLivSur.y has in Modelica the unit 1.
Output SurfaceComparison.refSur.TIntWalGarSur.y has in Modelica the unit 1.
Output SurfaceComparison.refSur.TWesWalGarSur.y has in Modelica the unit 1.
Output SurfaceComparison.refSur.TEasWalGarSur.y has in Modelica the unit 1.
Output SurfaceComparison.zonSur.TGarAir.y has in Modelica the unit 1.
Output SurfaceComparison.zonSur.TIntWalLivSur.y has in Modelica the unit 1.
Output SurfaceComparison.zonSur.TIntWalGarSur.y has in Modelica the unit 1.
Output SurfaceComparison.zonSur.TWesWalGarSur.y has in Modelica the unit 1.
Output SurfaceComparison.zonSur.TEasWalGarSur.y has in Modelica the unit 1.
Model: Buildings.ThermalZones.EnergyPlus_24_2_0.Validation.SurfaceComparison.SurfaceComparison
mwetter commented 3 weeks ago

@kbenne : Another new issue are the ERROR messages from fmi, which look like the xml writing changed: image

kbenne commented 2 weeks ago

Hi @mwetter. Here are corrections related to units and the causality attribute.

https://spawn.s3.us-east-1.amazonaws.com/custom/Spawn-light-0.6.0-0eed8d916f-Linux.tar.gz https://spawn.s3.us-east-1.amazonaws.com/custom/Spawn-light-0.6.0-0eed8d916f-win64.zip

I also validated the generated FMUs with fmpy's validator and there are a couple of warnings still remaining. I don't think that these remaining items are regressions, I think they are long standing, but in any case I am working on corrections and will issue another update within one or two days.

mwetter commented 2 weeks ago

@kbenne : Here is some feedback from testing with the latest E+ binaries.

  1. With this version I still get four entries of the form
    [ERROR][FMI2XML] Start attribute is required for this causality, variability and initial combination
    [ERROR][FMI2XML] Start attribute is required for this causality, variability and initial combination
    [ERROR][FMI2XML] Start attribute is required for this causality, variability and initial combination
    [ERROR][FMI2XML] Start attribute is required for this causality, variability and initial combination
    [ERROR][FMI2XML] Start attribute is required for this causality, variability and initial combination
    EnergyPlus Completed Successfully.

    for the model Buildings.ThermalZones.EnergyPlus_24_2_0.Examples.SingleFamilyHouse.AirHeating

For the SmallOffice this gives

[ERROR][FMI2XML] Start attribute is required for this causality, variability and initial combination
[ERROR][FMI2XML] Start attribute is required for this causality, variability and initial combination
[ERROR][FMI2XML] Start attribute is required for this causality, variability and initial combination
[ERROR][FMI2XML] Start attribute is required for this causality, variability and initial combination
[ERROR][FMI2XML] Start attribute is required for this causality, variability and initial combination
[ERROR][FMI2XML] Start attribute is required for this causality, variability and initial combination
[ERROR][FMI2XML] Start attribute is required for this causality, variability and initial combination
[ERROR][FMI2XML] Start attribute is required for this causality, variability and initial combination
[ERROR][FMI2XML] Start attribute is required for this causality, variability and initial combination
[ERROR][FMI2XML] Start attribute is required for this causality, variability and initial combination
[ERROR][FMI2XM
...(message truncated)

So let's fix this so we are not cluttering up the output and possible truncating real errors.

  1. In Unconditioned, I get a new warning

    431400.000 Unconditioned.building: Warning from EnergyPlus: ** Warning ** Zone 3 has multiple people objects with different PMV. 

    But there is only one People object in the idf file. Do you know what causes this and how to correct it?

  2. Regarding regressions, I see the following (summarized): image Above, one model uses Sunday the other Monday as the start day but both have now the same trajectory which is unexpected.

image Initial transients are slower, perhaps this is acceptable if it is related to the thermal mass warmup?

image Here, the zone air temperature of the garage now starts at 20 degC. This is an unconditioned zone (not connected to Modelica) and the outdoor temperature is -12 degC. It appears that the initialization is fixing the room temperature at 20 degC? This may also explain the above change in initial transients.

kbenne commented 1 week ago

Hello @mwetter, If you track the commit references in the above timeline I think I have addressed everything except the start day regression. I did add some additional tests, but I wasn't able to reproduce the issue. I used the "Living Zone Lights" value as a signal for where EnergyPlus is at in time, in an attempt to confirm that EnergyPlus is indeed where the FMU time api says it is. You can see the details of that testing approach here. I am interested in learning more about your testing strategy related to the start day so that I can attempt to reproduce.

mwetter commented 1 week ago

@kbenne : Not sure what causes the tests to work on your end but not in Modelica. My tests are by running

RunScript("/mnt/shared/Buildings/Resources/Scripts/Dymola/ThermalZones/EnergyPlus_24_2_0/Validation/RunPeriod/StartDayOfYear.mos");

This creates the figure image which are the electricity consumptions for the two E+ simulations, one having Sunday and the other Monday as the first day. (These are the trajectories red by the CI tests.) Both simulations start at t=0. The only difference is from the E+ side the entry

$ diff spawn-StartDayOfYear.sun/ModelicaBuildingsEnergyPlus.json spawn-StartDayOfYear.mon/ModelicaBuildingsEnergyPlus.json 
9c9
<     "start_day_of_year": "Sunday",
---
>     "start_day_of_year": "Monday",
31c31
<       "name": "/mnt/shared/Buildings/spawn-StartDayOfYear.sun/EnergyPlus.fmu",
---
>       "name": "/mnt/shared/Buildings/spawn-StartDayOfYear.mon/EnergyPlus.fmu",

When I run this with E+ 9.6.0, I get the correct trajectories, e.g.,

RunScript("/mnt/shared/Buildings/Resources/Scripts/Dymola/ThermalZones/EnergyPlus_9_6_0/Validation/RunPeriod/StartDayOfYear.mos");

produces image and the diff of ModelicaBuildingsEnergyPlus.json is as above.

kbenne commented 1 week ago

Oh! I know what it is. We changed the name of the parameter related to the start day in this commit, which was done after I branched and started working on the EnergyPlus update. I just need to bring this commit over.

mwetter commented 1 week ago

Good catch! I wondered why your tests work and mine won't...

kbenne commented 1 week ago

Here are the builds. I think this is a good point for you to run another round of tests. We might have another iteration, but I think this is very close unless something new pops up. I'm expecting that you will see a startup transient more aligned with the old behavior, but await your plots.

https://spawn.s3.us-east-1.amazonaws.com/custom/Spawn-light-0.6.0-9f1b36b00b-Linux.tar.gz https://spawn.s3.us-east-1.amazonaws.com/custom/Spawn-light-0.6.0-9f1b36b00b-win64.zip

mwetter commented 1 week ago

@kbenne : The results look good now. For SurfaceComparison the garage temperature (unconditioned) changed at the start of the simulation from around 12 degC to 4 degC, but this is not a concern as far as I can tell.

We still have the long standing issue that "EnergyPlus Completed Successfully" is interpreted as a warning: image I don't know why this is interpreted as a warning. Also, in OMEdit, it is in red font, whereas other E+ output is in black: image This would be nice to correct to avoid this confusing message.

The FMU does no longer trigger the warnings when loaded by Spawn, but the modelDescription.xml file is still incorrect (run through the FMU checker from the FMI Standard web page): image This should be easy to correct and important to avoid later incompatibility issues because the FMU does not conform to the FMI specification.