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

Allow ZoneHVAC:TerminalUnit:VariableRefrigerantFlow to be connected to AirloopHVAC and AirLoopHVAC:OutdoorAirSystem:EquipmentList #7605

Merged
merged 47 commits into from
Feb 13, 2020

Conversation

rraustad
Copy link
Contributor

@rraustad rraustad commented Nov 7, 2019

Pull request overview

New Feature: Connect split DX to VRF refrigerant loop

Manufacturers have recently allowed VRF split system DX fan coil units to be installed in air loops and outdoor air systems. This enhancement allows ZoneHVAC:TerminalUnit:VariableRefrigerantFlow terminal units to be located in the main air loop and outdoor air system and be connected to the AirConditioner:VariableRefrigerantFlow HVAC system.

Deliverables:

  • Initial NFP for air loop and OA Sys VRF coils
  • Design doc for mirroring plant configuration in air systems
  • Final NFP for air loop and OA Sys VRF coils
  • Code and unit tests
  • Documentation initial draft
  • Documentation final draft
  • Final PR code merge

Sub-tasks (☐ requires initial NFP and design approval):

  • Create a new class for air systems, DataHVACSystems.hh
  • Allow ZoneHVAC:TerminalUnit:VariableRefrigerantFlow object in main air loop
  • Allow ZoneHVAC:TerminalUnit:VariableRefrigerantFlow object in outside air system
  • Update unit tests with changes to class structures
  • add unit tests specific to model enhancement
  • docs
  • example file(s)

Testing:

  • Compare VRF in air loop with unitary system
    • Comparative plots of results
    • Annual energy use comparison
  • Compare VRF in OA system with DX or water coils
    • Comparative plots of results
    • Annual energy use comparison

NOTE: ENHANCEMENTS MUST FOLLOW A SUBMISSION PROCESS INCLUDING A FEATURE PROPOSAL AND DESIGN DOCUMENT PRIOR TO SUBMITTING CODE

Pull Request Author

Add to this list or remove from it as applicable. This is a simple templated set of guidelines.

  • Title of PR should be user-synopsis style (clearly understandable in a standalone changelog context)
  • Label the PR with at least one of: Defect, Refactoring, NewFeature, Performance, and/or DoNoPublish
  • Pull requests that impact EnergyPlus code must also include unit tests to cover enhancement or defect repair
  • Author should provide a "walkthrough" of relevant code changes using a GitHub code review comment process
  • If any diffs are expected, author must demonstrate they are justified using plots and descriptions
  • If changes fix a defect, the fix should be demonstrated in plots and descriptions
  • If any defect files are updated to a more recent version, upload new versions here or on DevSupport
  • If IDD requires transition, transition source, rules, ExpandObjects, and IDFs must be updated, and add IDDChange label
  • If structural output changes, add to output rules file and add OutputChange label
  • If adding/removing any LaTeX docs or figures, update that document's CMakeLists file dependencies

Reviewer

This will not be exhaustively relevant to every PR.

  • Perform a Code Review on GitHub
  • If branch is behind develop, merge develop and build locally to check for side effects of the merge
  • If defect, verify by running develop branch and reproducing defect, then running PR and reproducing fix
  • If feature, test running new feature, try creative ways to break it
  • CI status: all green or justified
  • Check that performance is not impacted (CI Linux results include performance check)
  • Run Unit Test(s) locally
  • Check any new function arguments for performance impacts
  • Verify IDF naming conventions and styles, memos and notes and defaults
  • If new idf included, locally check the err file and other outputs

New example file - VariableRefrigerantFlow_5Zone_wAirloop.idf (VRF TUs in zone, air loop and OA sys)
No warnings in new example file annual run.
New example file tested with AlgorithmType=FluidTCtrl VRF model as alternate to AlgorithmType=SysCurve model.

Existing example File Diffs:
1 example file has a slight difference in warmup day convergence results.

Unit Testing:
New unit test fixture added to ease unit testing - AirLoopFixture
New unit test - VRF_SysModel_inAirloop
SizingAnalysisObjectsTest.LoggingSubStep4stepPerHour unit test is occasionally hanging when all unit tests are run locally.

@rraustad
Copy link
Contributor Author

rraustad commented Nov 22, 2019

Preliminary test to see what changes are required to allow the ZoneHVAC:TU:VRF object in the air loop. the Furnace example file doesn't look like it's controlling very well. I replaced the furnace in this file with ZoneHVAC:TerminalUnit:VariableRefrigerantFlow and associated coils. Also changed the FanAndCoilAvailSched to turn off at hr 18 (instead of 17), to match the lighting schedule.

image

@rraustad rraustad added the NewFeature Includes code to add a new feature to EnergyPlus label Dec 6, 2019
Changes to the IDD appear to be minimal as no new objects will be added to E+. The only changes to model inputs are the allowed location of the TU object and thermostat location for controlling air loop equipment.

```
ZoneHVAC:TerminalUnit:VariableRefrigerantFlow,
Copy link
Contributor

Choose a reason for hiding this comment

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

It bothers me that we'll now have a "ZoneHVAC" object that's valid in airloops. We've already set the reverse precedent by allowing "AirloopHVAC:UnitarySystem" to be in airloops and in ZoneHVAC. Will any other components come along that are applicable in both places? Do we want two flavors "ZoneHVAC:TerminalUnit:VariableRefrigerantFlow" and "AirloopHVAC:TerminalUnit:VariableRefrigerantFlow". Or better yet, why not allow "Coil:Cooling:DX:VariableRefrigerantFlow" and Heating as valid coil type in AirloopHVAC:UnitarySystem? Then you don't need any new control logic etc. (Sorry this is a bit late in the game.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm all for changing the names of these objects to reflect system type instead of location. Instead of AirloopHVAC:UnitarySystem it should be just UnitarySystem or HVAC:UnitarySystem. Same for the ZoneHVAC VRF terminal unit. That's a different discussion.

Multiple flavors of any object is no longer acceptable. Although that is managed nicely in Furnaces.cc so it is possible if that's the route to take. It would be nice if JSON could handle that directly.

Furnaces::GetFurnaceInput

//       Furnace and UnitarySystem objects are both read in here.
//       Will still have 2 differently named objects for the user, but read in with 1 DO loop.
if (HeatOnlyNum <= NumHeatOnly) {
    CurrentModuleObject = "AirLoopHVAC:Unitary:Furnace:HeatOnly";
    FurnaceType_Num = Furnace_HeatOnly;
    GetObjectNum = HeatOnlyNum;
} else {
    CurrentModuleObject = "AirLoopHVAC:UnitaryHeatOnly";
    FurnaceType_Num = UnitarySys_HeatOnly;
    GetObjectNum = HeatOnlyNum - NumHeatOnly;
}

Regarding the VRF coils, the condensing unit connects to (via the TerminalUnitList) and talks to the terminal units (not the coils) to sum cooling/heating rates, limit coil capacity, to know when all TUs have been simulated, and maybe other parameters. If I remove the coils from the terminal unit I'll loose that connection (which means rethinking the whole VRF model communication).

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I get all that. So, the terminal unit has inputs for an OA mixer - how will that play out when used in an airloop?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested OAMixer flows are 0 and autosizing will create 0 flow. Same as ChangeoverBypass. This may mean a new object, one without an internal OA mixer, is not all that out of the question. And I suppose I could drop a VRF coil object on the main branch and create the TU in the background and add that TU to the correct TU list and everything would work the same as it does now.

Copy link
Contributor

Choose a reason for hiding this comment

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

I wouldn't get into making shadows object in the background. I thought maybe you'd let the OA mixer be active in the TU and then the user wouldn't need an OA system beyond what's in the VRF terminal unit. Would that work? Or let the OA mixer be optional and only require it when it's used as zone equipment. This may run into the same input processing/init checking problems that UnitarySystem has with its dual role.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree on shadow objects. For child components on the main branch, a parent has to control it (or that control is within the child model at a higher level). So let's assume the parent will be the TU for now. As for an internal OA mixer (w/o economizer) vs OA system mixer, I agree that it could be one or the other. But both would be a problem in sizing (unless of course that was somehow included in sizing). The subject of optional OA mixer came up with ChangeoverBypass and I don't recall right now how I left that (I think I tried doing a node copy from OA mixer inlet to outlet to avoid calling the mixer). And yes I am running into the same problem of zone equipment being called first for a dual role object (i.e., GetInput doesn't have all the info yet). It would be much easier for models that use the OO method of a "new" instance to avoid some of this "waiting for info" issue but that I think is a major rewrite. I'm trying to move in that direction but I should take these types of steps slowly and methodically.

Copy link
Contributor

Choose a reason for hiding this comment

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

For the zone vs airloop input processing, I think if GetInput were to just populate an array of input data and save all the validation and cross-checking for an Init step, that should solve the issue of timing.

@rraustad rraustad added the IDDChange Code changes impact the IDD file (cannot be merged after IO freeze) label Jan 8, 2020
@rraustad rraustad changed the title NFP Connect split DX to VRF refrigerant loop Allow ZoneHVAC:TerminalUnit:VariableRefrigerantFlow to be connected to AirloopHVAC and AirLoopHVAC:OutdoorAirSystem:EquipmentList Jan 17, 2020
\reference-class-name validBranchEquipmentTypes
\reference validBranchEquipmentNames
\reference-class-name validOASysEquipmentTypes
\reference validOASysEquipmentNames
Copy link
Contributor Author

Choose a reason for hiding this comment

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

TU now allowed on main branch and in OA system

Copy link
Contributor

Choose a reason for hiding this comment

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

Update the object \memo to mention the new applications.

@@ -40246,14 +40250,12 @@ ZoneHVAC:TerminalUnit:VariableRefrigerantFlow,
\note This field is set to zero flow when the VRF terminal unit is connected to
\note central dedicated outdoor air through air terminal single duct mixer object.
A5 , \field Supply Air Fan Operating Mode Schedule Name
\required-field
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This and next 2 fields are (will be) removed so that air loop and OA system TUs do not require a fan object.

\type real
\maximum 21.0
\default 21.0
\units C
\note Supplemental heater will not operate when outdoor temperature exceeds this value.
A19; \field Controlling Zone or Thermostat Location
Copy link
Contributor Author

Choose a reason for hiding this comment

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

And finally air loop equipment need to know the controlling zone otherwise the TU will be set point controlled.

OutsideAirSys(thisDOAS.m_OASystemNum).compPointer[CompNum]->AirInNode;
OutsideAirSys(thisDOAS.m_OASystemNum)
.compPointer[CompNum]
->getAirInNode(OutsideAirSys(thisDOAS.m_OASystemNum).ComponentName(CompNum), 0);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Switched from a UnitarySys pointer with direct access to UnitarySys member data to DataHVACSystem::HVACSystem data pointer with member function to access air inlet node.

}
OutsideAirSys(thisDOAS.m_OASystemNum).OutletNodeNum(CompNum) = HVACVariableRefrigerantFlow::GetVRFTUOutAirNodeFromName(
OutsideAirSys(thisDOAS.m_OASystemNum).ComponentName(CompNum), errorsFound);
if (errorsFound) OutletNodeErrFlag = true;
Copy link
Contributor Author

@rraustad rraustad Jan 23, 2020

Choose a reason for hiding this comment

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

Also added new mining function to get air inlet/outlet nodes. I guess this could have used the same pointer type call as above (if that pointer were available at this time in calling order).

@rraustad
Copy link
Contributor Author

rraustad commented Feb 3, 2020

This comparison of using ZoneHVAC:TerminalUnit:VariableRefrigerantFlow in the air loop as a substitute for a UnitarySystem looks pretty good. Dotted lines always represent VRF and solid lines the existing HP example file. Coils are outputting similar capacity while holding the zone temperature at Tstat set point. However, the zone relative humidity is different between these 2 coil models since the DX coil model cycles refrigerant flow through the coil and the VRF model modulates refrigerant flow through the coil (i.e., the VRF coil is active the entire time step).

image

The coil electric power report shows similar energy use between the 2 models. Although the DX cooling coil model using cycling refrigerant flow and the VRF model uses modulated refrigerant flow, the refrigerant flow over the time step should be similar if the same sensible load is met.

image

The coil runtime fractions are different in cooling mode since the VRF model uses a custom function while the VRF heating coil uses the original DX heating coil function. I did expect the heating RTF to be closer between these models and have tried to align the performance curves as best I can. There is an issue with differences in defrost mode so maybe that is causing the difference in heating RTF. I will keep looking into this discrepancy.

image

Update: Defrost power is now similar. The VRFHeatCapFT was malformed in my attempt to make them equal. Also note I was looking at heating coil RTF and VRF defrost is multiplied by VRF condenser RTF. The heating coil and VRF runtime fractions are now equal.

image

The differences in RTF cause differences in main duct air flow and OA flow yet the mixed air node temperatures are identical. This also does not make sense and I will have to investigate this further.

image

Update: the mixed air node temperatures are correct since the air loop and OA flow rates are proportionally different.

image

Annual energy use is of course different but also close. I will still try to find reasons for these differences.

CompareVRFtoHPinAirloop

Update: The correction to VRFHeatCapFT caused the annual energy use to converge on that reported by the heat pump.

CompareVRFtoHPinAirloop2

@rraustad
Copy link
Contributor Author

rraustad commented Feb 6, 2020

This test required an IDD change to allow a Master Thermostat Priority Control Type = None since the only TU connected to the VRF system is set point controlled (I will try to revert that out today but it seems like a good input for users, otherwise this field will not be used for SP control only).

Update: No IDD change required.

This example compares 5ZoneAirCooled with a VRF TU in the OA system. The 2 water coils were replaced with a VRF TU. The SPs were not changed and the control shows similar results for the OA air stream temperature control. The water heating coil is first in the OA system and then the water cooling coil. For the VRF TU, the cooling coil is required to be upstream of the heating coil and that restriction was not changed at this time (so the VRF cooling coil is first then the heating coil). The same performance curves were used in this example is for the UnitarySystem_HeatPumpAuto test above.

Update: there is a serious issue with modulating OA when using DX coils. Low air flow through coils will cause very low or high outlet temperatures. I was able to make a comparison by setting the min/max OA flow in the controller to 0.25 (a constant OA system). I am still trying to figure out how to allow DX coils to be used with variable flow OA systems. The CoilSystem:*:DX must have similar issues.

image

@rraustad
Copy link
Contributor Author

rraustad commented Feb 7, 2020

Well, I was getting a lot of DX coil warnings on air flow per capacity with the settings I had. I had a low air flow per watt since I was trying to get the VRF coils to meet the OA loads. This is probably not the right way to approach this so I revised the inputs so the air flow per watt warnings were gone. There is a gas supplemental heater so for heating loads the results are reasonable but for cooling loads the DX coil can't handle the high load. The simulation is asking the DX cooling coil to pull 30+ C outdoor air down to 11.5 C. This is asking quite a lot when typical DX coil delta T's are around 10 C.

image

And with these new inputs I checked the difference in annual energy use.

Total site energy use went up about 1.5%. Not sure if that sound right when substituting DX for water coils except that at extreme outdoor temperatures the DX equipment may have a lower COP than a chiller or boiler (just another thing to look at). The end use natural gas energy was reduced. This shows the boiler was unloaded which makes sense. End use cooling energy went up. With a 3.2 COP on the chiller and 3.0 on the VRF system (just what inputs where being used) this also makes sense. Plant pump energy also went down since there are less water coils to serve. Fan electricity did show a change which is a little odd given that both simulations should have the same OA flow rates.

Update: A change in fan energy is likely due to different mixed air temperatures since the DX coil can't maintain the OA system cooling set point as well as water coils.

There is now an Other category that I assume is reporting the VRF electricity and supp gas heater gas use.

CompareVRFto5ZoneAirCooled

@rraustad
Copy link
Contributor Author

Finally getting the Fluid temperature VRF model to behave. Top left, OA system VRF using SP control. Top right, zone 5 temperature control with VRF TU in air loop serving all 5 zones without VRF zone TU in zone 5. Bottom left, TU inlet/outlet flow rates now match. This was fixed in #7295 but somehow broke again. Bottom right, zone 2 temperature control with air loop providing conditioned air and zone TU picking up remaining load.

image

@rraustad
Copy link
Contributor Author

Ready for final review. This has been tested quite extensively. Both VRF models allow air loop and OA system TUs. Final code changes will be based on 1) comments, 2) clang-formatting VRF code, and 3) removing any extraneous comment lines (e.g. SystemFan calls). I still feel there may be an issue with air loop availability managers but haven't looked at that yet (struck me last night with the system fan call changes).

\paragraph{Field: Controlling Zone or Thermostat Location}\label{controlling-zone-or-thermostat-location-2-110}

This alpha field is used only for air loop equipment and defines the controlling zone or thermostat location for load based operational control. This field is not required for zone equipment or equipment used in outdoor air systems. Leave this field blank for zone equipment, outdoor air equipment and air loop equipment that is terminal unit or coil air outlet controlled based on a set point termperature. When the terminal unit is used in an air loop and this terminal unit is load controlled, this zone's thermostat will control operation.

Copy link
Member

Choose a reason for hiding this comment

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

@mjwitte are the latest changes satisfactory?

CompName, FirstHVACIteration, AirLoopNum, CompIndex, HeatingActive, CoolingActive, CompIndex, OAUCoilOutTemp, ZoneEquipFlag);
Real64 sensOut = 0.0;
Real64 latOut = 0.0;
OutsideAirSys(OASysNum).compPointer[CompIndex]->simulate(CompName,
Copy link
Member

Choose a reason for hiding this comment

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

The performance hit here will probably be undetectable, but it would probably be best to declare them as static const if those are able to be constant.

CompName, FirstHVACIteration, AirLoopNum, CompIndex, HeatingActive, CoolingActive, CompIndex, OAUCoilOutTemp, ZoneEquipFlag);
Real64 sensOut = 0.0;
Real64 latOut = 0.0;
OutsideAirSys(OASysNum).compPointer[CompIndex]->simulate(CompName,
Copy link
Member

Choose a reason for hiding this comment

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

However, I'm glad you are thinking about it.

@rraustad
Copy link
Contributor Author

The last thing I will do is clang-format. So if there are some minor changes (e.g., Real64 static const sensOut = 0.0) I am happy to apply on final commit.

@Myoldmopar
Copy link
Member

@rraustad this looks really good. I like that you have pushed into some new territory with the new HVAC base class. It will be very interesting to see where that leads as we try to expand that into new components. CI is super happy, just one very tiny diff. Your new unit test modifications are great.

Marking those variables as static const is fine as long as the functions don't expect non-const references to be passed in. I'm not super concerned about that right now.

If you want to run Clang-format, that is fine, but you can also just hold off from doing that for now. We've got a lot of PRs to get merged in and unnecessary conflicts could get real annoying in crunch time.

More importantly, running your new input file has the following warning, are you ok with this:

GetAirPathData: AirLoopHVAC="MAIN AIRLOOP" has no Controllers.

I know you may want to investigate some things a little more regarding the availability managers and that's fine, but given the IO freeze, I think we should just wrap up these last couple things and get this in.

@mjwitte final concerns/thoughts?

@rraustad
Copy link
Contributor Author

rraustad commented Feb 13, 2020

  1. GetAirPathData: AirLoopHVAC="MAIN AIRLOOP" has no Controllers. is normal for non-water coil based air loops
  2. I don't need to clang-format the file now, could do this after release
  3. Avail managers need to be fixed for VRF (previously only zone eq) and UnitarySystem (originally only air loop eq)
  4. I can create a follow-up issue to this branch and finish these minor issues up there

@Myoldmopar
Copy link
Member

4. I can create a follow-up issue to this branch and finish these minor issues up there

Make it so. @mjwitte let me know how you're feeling about this since you had a number of comments along the way.

@rraustad
Copy link
Contributor Author

Follow up issue #7773 to clean up any remaining comments/code changes/etc.

@Myoldmopar
Copy link
Member

Thanks for making that issue @rraustad, @mjwitte if you find anything else that needs addressing we'll just add it to the follow-up issue. I'm ready for this to go in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
IDDChange Code changes impact the IDD file (cannot be merged after IO freeze) NewFeature Includes code to add a new feature to EnergyPlus
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants