lbl-srg / modelica-buildings

Modelica Buildings library
255 stars 159 forks source link

add Windows binaries for Spawn #2132

Closed mwetter closed 4 years ago

mwetter commented 4 years ago

This issue is to add Windows binaries to Spawn of EnergyPlus.

mwetter commented 4 years ago

@kbenne : I get an error when unloading the dll on Windows. The code on this branch works on Linux with

jm_ipython.sh jmodelica.py  Buildings.ThermalZones.EnergyPlus.Validation.ThermalZone.OneZone

and it also works in Dymola on Linux.

When I run this model in Dymola on Windows, I get

Unkown error
fmi2Terminate returned with status Error for building OneZone.building.

The place where the error is triggered is https://github.com/lbl-srg/modelica-buildings/blob/issue2132_spawn_windows/Buildings/Resources/src/ThermalZones/EnergyPlus/C-Sources/EnergyPlusFMU.c#L327 which returns a non OK status.

I committed the code, except for the Windows E+ binaries. You can install them by running (on any OS)

cd Buildings/Resources/src/ThermalZones/EnergyPlus
python install.sh

Can you please check why I don't get OK when calling fmi2_import_terminate

kbenne commented 4 years ago

@mwetter

  1. Do you know where the message "Unkown error" originates? The typo should make it easy to find unless it is in third party code. I grepped for it in my code with no result.

  2. Given that terminate normally happens at the end of the simulation can you tell if the simulation was able to startup and begin an EnergyPlus simulation? You should have output files even if they are incomplete. And before that, an EnergyPlus FMU should have been generated.

mwetter commented 4 years ago

@kbenne : The error message is "Unknown error" (my typo in this issue only). I do not have code that has this message, and neither does the fmi-library, so it either comes from your code or from Windows.

The FMU is generated, but I see no output files of EnergyPlus or no sign that it started.

mwetter commented 4 years ago

@kbenne : The line that I added in https://github.com/lbl-srg/modelica-buildings/commit/f73cd4e8e0ee8117bcfa282eb9cd3e5540b7aa1c is not reached on Windows. The call above seems to issue "Unknown error". Afterwards, Dymola calls the destructor function, e.g., I see "Entered FMUBuildingFree" in dslog.txt.

Does this work in your CI testing on Windows?

kbenne commented 4 years ago

@mwetter https://github.com/NREL/Spawn/commit/b630852af9ec7338c47dfbedcd25aaad32c87469 yields successful simulation on windows. you will need a new epfmi.dll

mwetter commented 4 years ago

@kbenne : Thanks, I will test next week. Have the results been confirmed as discussed in https://github.com/lbl-srg/modelica-buildings/issues/1954#issuecomment-696933108?

mwetter commented 4 years ago

@kbenne : The new binaries still don't work on Windows. Running them in Dymola gives the same error. When I run dymosim.exe from the Windows prompt, I get:

(base) E:\modelica-buildings\Buildings>dymosim.exe
dymosim.exe started
... "dsin.txt" loading (dymosim input file)
[INFO][FMILIB] XML specifies FMI standard version 2.0
[INFO][FMILIB] Loading 'win64' binary with 'default' platform types
Cannot parse json: '/E:/modelica-buildings/Buildings/tmp-simulation-OneZone.building/resources\../model.spawn'
Simulation successfully terminated

The file E:\modelica-buildings\Buildings\tmp-simulation-OneZone.building\resources\..\model.spawn exists (note the corrected slashes), and the message Cannot parse json... is generated during the call to fmi2_import_instantiate.

Can you please

  1. try to reproduce this on your end,
  2. check where the Simulation successfully terminated comes from,
  3. make sure the call to fmi2_import_instantiate returns an error status if an error occurs, and
  4. let me know if/when you fixed it and posted new binaries.

Once we understand what the root cause is, we need to see how to tighten up the CI testing so we catch such issues earlier. This took many hours if not days of debugging and is an open issue since more than a month.

Also, per the FMI standard, model.spawn should be in resources if it is a resource that is needed by the fmu.

kbenne commented 4 years ago

You must be doing it wrong because it "works for me" (TM). There error your describe is exactly what I saw before this change https://github.com/NREL/Spawn/commit/b630852af9ec7338c47dfbedcd25aaad32c87469. I documented the root cause in the code comments of the commit. You can read about it for yourself. That said, yes I can probably figure out why you are getting a different result. It smells like you do not have the version of binaries I intended you to have. Could be that I posted an incorrect version, but I'm not sure yet.

The problem is with your process as you are checking in binaries and there are manual steps that get out of sync between the three repositories, which include this one, NREL/spawn, and NREL/EnergyPlus. I have enabled and indulged this for your convenience long enough. Your process is extremely brittle. I manage and avoid the problems that you are having by putting NREL/spawn at the top and having MBL and EnergyPlus as git submodules which are then built as cmake sub projects. See here https://github.com/NREL/Spawn/blob/develop/CMakeLists.txt#L196. This way no matter what branch of NREL/spawn you are on, you have the correct version of the sub modules, you build from the top and everything is in sync. I suggest you take some time to check out NREL/spawn and learn how to build from the top down. I can help you through this when you have time.

Unlike the other fmi2 functions, fmi2Instantiate which is the FMI instantiate function returns a pointer to the fmi component not a status message. See here https://github.com/NREL/Spawn/blob/1ad59a6dbfb6b7f5f87d9c55954bc3692bc8343c/fmi2/fmi2FunctionTypes.h#L165 and https://github.com/NREL/Spawn/blob/b630852af9ec7338c47dfbedcd25aaad32c87469/epfmi/EPFMI.cpp#L91. I should probably catch exceptions and return a null pointer which will help issues like this, but it will not prevent the possibility of a segfault getting through. I suspect the functions that you are using to interface with the FMU (fmi2_import_instantiate) will interpret the null pointer as an error and give you some feedback.

The lack of feedback about errors (and C++ exceptions) is addressed pretty well after instantiate is complete. I trap exceptions and feed them through the log and the return error code, the problem is that this system is still being setup during instantiate.

mwetter commented 4 years ago

@kbenne : The binaries are pulled from here: https://github.com/lbl-srg/modelica-buildings/blob/issue2132_spawn_windows/Buildings/Resources/src/ThermalZones/EnergyPlus/install.py#L93 and they are

_Version: 0.0.1_

_Build: 316d0c8c19_

Is this what you meant to share? I don't see what is brittle about pulling them from a place where you tell me they are, unless you want to version them and have me put the version number or commit into install.py. Building binaries for Linux, Windows and OS X is something you set up on your end.

fmi2Instantiate is defined in the FMI standard as "The function returns a new instance of an FMU. If a null pointer is returned, then instantiation failed." It seems it does not return null, see

jm_status_enu_t fmi2_import_instantiate(fmi2_import_t* fmu,
  fmi2_string_t instanceName, fmi2_type_t fmuType,
  fmi2_string_t fmuResourceLocation, fmi2_boolean_t visible) {
    fmi2_string_t fmuGUID = fmi2_import_get_GUID(fmu);
    fmi2_boolean_t loggingOn = (fmu->callbacks->log_level > jm_log_level_nothing);
    fmi2_component_t c;
    if(!fmuResourceLocation) 
        fmuResourceLocation = fmu->resourceLocation;
    c = fmi2_capi_instantiate(fmu -> capi, instanceName, fmuType, fmuGUID,
                              fmuResourceLocation, visible, loggingOn);
    if (c == NULL) {
        return jm_status_error;
    } else {
        return jm_status_success;
    }
}

If it were to return NULL then the right status would be returned by fmi2_import_instantiate.

mwetter commented 4 years ago

Todo: