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

Round trip forward and reverse translation issue. #4096

Closed
DavidGoldwasser opened this issue Sep 28, 2020 · 5 comments · Fixed by #4097
Closed

Round trip forward and reverse translation issue. #4096

DavidGoldwasser opened this issue Sep 28, 2020 · 5 comments · Fixed by #4097

Comments

@DavidGoldwasser
Copy link
Collaborator

DavidGoldwasser commented Sep 28, 2020

I took out could from this measure to isolate the issue. The example model used is ExampleModel.osm
https://github.com/NREL/openstudio-common-measures-gem/tree/develop/lib/measures/view_data/tests

Note, the model in the repo is version 1.3, but I have used OS App to upgrade to 3.0.1 here.
https://drive.google.com/file/d/1BC11cHIJCtfC6K3jMmO45WoLwMcq9Omq/view?usp=sharing

I see the following possible casues.

  1. The file is invalid and there is no bug (I would need to fix the test model so it isn't invalide)

  2. Version translation from earlier version to 3.1.0 is broken

  3. Forward translation to IDF is broken

  4. Reverse translation back to OSM is broken.

`
require 'openstudio'

vt = OpenStudio::OSVersion::VersionTranslator.new
model = vt.loadModel('ExampleModel.osm')
model = model.get

forward translate OSM file to IDF file

ft = OpenStudio::EnergyPlus::ForwardTranslator.new
workspace = ft.translateModel(model)
workspace.save('my.idf', true)

reverse translate IDF file to OSM file

rt = OpenStudio::EnergyPlus::ReverseTranslator.new
model2 = rt.translateWorkspace(workspace)
model2.save('my.osm', true)
`

The error that shows up on the second to last line rt.translateWorkspace(workspace) is
Error: Unknown OpenStudio Enum Value '' in /Users/dgoldwas/Documents/GitHub/URBANopt/openstudio-common-measures-gem/lib/measures/view_data/tests/round_trip_check.rb:14:in translateWorkspace'`

@DavidGoldwasser DavidGoldwasser added the Triage Issue needs to be assessed and labeled, further information on reported might be needed label Sep 28, 2020
@DavidGoldwasser
Copy link
Collaborator Author

I should point out when I used a different model in the measure test directory SimpleModel.osm I didn't see this issue, so that doesn't determine if the model is invalid, but the issue is at least model specific.

@DavidGoldwasser
Copy link
Collaborator Author

DavidGoldwasser commented Sep 28, 2020

Some more details. This test did not fail in OpenStudio 3.0.1. with the same ExampleModel.osm as the test model.

One odd thing I see in version translation of OSM file to 3.0.1 is both items being cleared out from availability manager list, however this didn't seem to be a problem with 3.0.1, so maybe it isn't an issue.
Screen Shot 2020-09-28 at 11 19 41 AM

Here is what is looks like in the 9.4 IDF
`AvailabilityManagerAssignmentList,
Air Loop HVAC 1Availability Manager List, !- Name
AvailabilityManager:Scheduled, !- Availability Manager Object Type 1
Air Loop HVAC 1 Availability Manager; !- Availability Manager Name 1

AvailabilityManager:Scheduled,
Air Loop HVAC 1 Availability Manager, !- Name
Always On Discrete; !- Schedule Name`

@jmarrec
Copy link
Collaborator

jmarrec commented Sep 28, 2020

(lldb) n
[openstudio.model.YearDescription] <1> 'UseWeatherFile' is not yet a supported option for YearDescription
Process 7473 stopped
* thread #1, name = 'openstudio', stop reason = breakpoint 1.5
    frame #0: 0x0000555555e177ef openstudio`EnumBase<openstudio::IddObjectType>::lookupValue(this=0x00007fffffff8700, t_name="") at EnumBase.hpp:150
   147 	        {
   148 	          return itr->second;
   149 	        }
-> 150 	        throw std::runtime_error("Unknown OpenStudio Enum Value '" + t_name + "'");
   151 	      }
   152 	
   153 	      /** Returns t_value if it is in the domain. Otherwise throws std::runtime_error. */
(lldb) bt
* thread #1, name = 'openstudio', stop reason = breakpoint 1.5
  * frame #0: 0x0000555555e177ef openstudio`EnumBase<openstudio::IddObjectType>::lookupValue(this=0x00007fffffff8700, t_name="") at EnumBase.hpp:150
    frame #1: 0x0000555555e0c100 openstudio`EnumBase<openstudio::IddObjectType>::EnumBase(this=0x00007fffffff8700, t_value="\xe8\x89\xff\xff\xff\U0000007f"...) at EnumBase.hpp:62
    frame #2: 0x0000555555e0b19d openstudio`openstudio::IddObjectType::IddObjectType(this=0x00007fffffff8700, t_name="\xe8\x89\xff\xff\xff\U0000007f"...) at IddEnums.hpp:120
    frame #3: 0x000055555b715784 openstudio`operator(__closure=0x00007fffffff88a0, _thermostatType=0x00007fffffff89d0, _thermostatName=0x00007fffffff8a00) at ReverseTranslateZone.cpp:216
    frame #4: 0x000055555b71663c openstudio`openstudio::energyplus::ReverseTranslator::translateZone(this=0x000055556622b990, workspaceObject=0x000055556623b608) at ReverseTranslateZone.cpp:248
    frame #5: 0x000055555b690951 openstudio`openstudio::energyplus::ReverseTranslator::translateAndMapWorkspaceObject(this=0x000055556622b990, workspaceObject=0x000055556623b608) at ReverseTranslator.cpp:1079
    frame #6: 0x000055555b68ddc6 openstudio`openstudio::energyplus::ReverseTranslator::translateWorkspace(this=0x000055556622b990, workspace=0x0000555564fa7110, progressBar=0x0000000000000000, clearLogSink=true) at ReverseTranslator.cpp:222

(lldb) fr select 3
frame #3: 0x000055555b715784 openstudio`operator(__closure=0x00007fffffff88a0, _thermostatType=0x00007fffffff89d0, _thermostatName=0x00007fffffff8a00) at ReverseTranslateZone.cpp:216
   213 	          auto checkIfDualSetpointAndTranslate = [&](boost::optional<std::string>& _thermostatType, boost::optional<std::string>& _thermostatName) {
   214 	            if( _thermostatName && _thermostatType ) {
   215 	              boost::optional<WorkspaceObject> _i_thermostat = workspace.getObjectByTypeAndName(IddObjectType(_thermostatType.get()),
-> 216 	                                                                                                _thermostatName.get());
   217 	              if (_i_thermostat) {
   218 	                boost::optional<ModelObject> _m_thermostat = translateAndMapWorkspaceObject(_i_thermostat.get());
   219 	                if( _m_thermostat )
(lldb) p _thermostatName.get()
(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >) $1 = ""
(lldb) p _thermostatType.get()
(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >) $2 = ""
(lldb) 

(lldb) fr select 4
frame #4: 0x000055555b71663c openstudio`openstudio::energyplus::ReverseTranslator::translateZone(this=0x000055556622b990, workspaceObject=0x000055556623b608) at ReverseTranslateZone.cpp:248
   245 	          {
   246 	            boost::optional<std::string> thermostatType = _zoneControlThermostat.getString(ZoneControl_ThermostatFields::Control2ObjectType);
   247 	            boost::optional<std::string> thermostatName = _zoneControlThermostat.getString(ZoneControl_ThermostatFields::Control2Name);
-> 248 	            checkIfDualSetpointAndTranslate(thermostatType, thermostatName);
   249 	          }
   250 	
   251 	          // Group 3
(lldb) p thermostatType.get()
(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >) $3 = ""
(lldb) p thermostatName.get()
(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >) $4 = ""
(lldb) p _zoneControlThermostat.get()
error: <user expression 5>:1:24: no member named 'get' in 'openstudio::WorkspaceObject'
_zoneControlThermostat.get()
~~~~~~~~~~~~~~~~~~~~~~ ^
(lldb) p _zoneControlThermostat.nameString()
error: <user expression 6>:1:35: too few arguments to function call, expected 1, have 0
_zoneControlThermostat.nameString()
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^
note: 'nameString' declared here

(lldb) p _zoneControlThermostat.nameString(false)
(std::__cxx11::string) $5 = {
  _M_dataplus = (_M_p = "Thermal Zone 1 Thermostat")
  _M_string_length = 25
   = (_M_local_buf = char [16] @ 0x0000000037068170, _M_allocated_capacity = 25)
}

@jmarrec
Copy link
Collaborator

jmarrec commented Sep 28, 2020

Ok I see what's going on:

ZoneControl:Thermostat,
  Thermal Zone 1 Thermostat,              !- Name
  Thermal Zone 1,                         !- Zone or ZoneList Name
  Thermal Zone 1 Thermostat Schedule,     !- Control Type Schedule Name
  ThermostatSetpoint:DualSetpoint,        !- Control 1 Object Type
  Thermostat Setpoint Dual Setpoint 1,    !- Control 1 Name
  ,                                       !- Control 2 Object Type
  ,                                       !- Control 2 Name
  ,                                       !- Control 3 Object Type
  ,                                       !- Control 3 Name
  ,                                       !- Control 4 Object Type
  ,                                       !- Control 4 Name
  0;                                      !- Temperature Difference Between Cutout And Setpoint {deltaC}

We're getting a problem here because E+ decided to use a numeric field at the end of what looks like an extensible thing. At the time of writing the RT for ReverseTranslateZone.cpp, I suppose this wasn't an option, so no-one ever double checked that we wouldn't hit a problem with empty fields. Anyways, this is a problem of the using the right getString overload one that would return uninitializedEmpty.

// Group 1
{
boost::optional<std::string> thermostatType = _zoneControlThermostat.getString(ZoneControl_ThermostatFields::Control1ObjectType);
boost::optional<std::string> thermostatName = _zoneControlThermostat.getString(ZoneControl_ThermostatFields::Control1Name);
checkIfDualSetpointAndTranslate(thermostatType, thermostatName);
}
// Group 2
{
boost::optional<std::string> thermostatType = _zoneControlThermostat.getString(ZoneControl_ThermostatFields::Control2ObjectType);
boost::optional<std::string> thermostatName = _zoneControlThermostat.getString(ZoneControl_ThermostatFields::Control2Name);
checkIfDualSetpointAndTranslate(thermostatType, thermostatName);
}
// Group 3
{
boost::optional<std::string> thermostatType = _zoneControlThermostat.getString(ZoneControl_ThermostatFields::Control3ObjectType);
boost::optional<std::string> thermostatName = _zoneControlThermostat.getString(ZoneControl_ThermostatFields::Control3Name);
checkIfDualSetpointAndTranslate(thermostatType, thermostatName);
}
// Group 4
{
boost::optional<std::string> thermostatType = _zoneControlThermostat.getString(ZoneControl_ThermostatFields::Control4ObjectType);
boost::optional<std::string> thermostatName = _zoneControlThermostat.getString(ZoneControl_ThermostatFields::Control4Name);
checkIfDualSetpointAndTranslate(thermostatType, thermostatName);
}

@jmarrec jmarrec added component - IDF Translation severity - Normal Bug and removed Triage Issue needs to be assessed and labeled, further information on reported might be needed labels Sep 28, 2020
jmarrec added a commit that referenced this issue Sep 28, 2020
There is a numeric field after the Groups 1-4, so handle blanks correctly.
@DavidGoldwasser
Copy link
Collaborator Author

@jmarrec that commit did the trick. I ran both the test code above and the full measure test.

jmarrec added a commit that referenced this issue Sep 29, 2020
There is a numeric field after the Groups 1-4, so handle blanks correctly.
jmarrec added a commit that referenced this issue Sep 29, 2020
jmarrec added a commit that referenced this issue Sep 29, 2020
There is a numeric field after the Groups 1-4, so handle blanks correctly.
@jmarrec jmarrec mentioned this issue Sep 29, 2020
20 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants