Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Address zone air terminal zone outdoor air flow rate reporting issues #7955

Merged
merged 13 commits into from
Jun 2, 2020

Conversation

Nigusse
Copy link
Contributor

@Nigusse Nigusse commented Apr 29, 2020

Pull request overview

  • Fixes AirTerminal:SingleDuct:ConstantVolume:NoReheat Zone Air Terminal Outdoor Air Volume Flow Rate always zero #7950
  • Fixes the Zone Air Terminal Outdoor Air Volume Flow Rate reporting issues for single duct constant volume with and with NoReheat air terminals.
  • Zone Air Terminal Outdoor Air Volume Flow Rate is new reporting variable for AirTerminal:SingleDuct:ConstantVolume:Reheat air terminal object and it is assigning a value for existing variable for AirTerminal:SingleDuct:ConstantVolume:NoReheat air terminal object
  • Consolidated the update equation in a common new member function for single duct air terminals.
  • Updated I/O Reference documentation
  • Diffs is expected in test files with AirTerminal:SingleDuct:ConstantVolume:NoReheat object and this report variable specified.

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

@Nigusse Nigusse added the Defect Includes code to repair a defect in EnergyPlus label Apr 29, 2020
@Nigusse
Copy link
Contributor Author

Nigusse commented Apr 29, 2020

Below is plots of Zone Air Terminal Outdoor Air Volume Flow Rate report variable for AirTerminal:SingleDuct:ConstantVolume:NoReheat air terminal object for develop and this branch. Develop shows zero flow rate because this variable was not assigned a value. This was based on a test file ASHRAE9012016_RestaurantFastFood_Denver.idf. This test file is added as a defect file.

ZoneAirTerminalOAVolumeFlowRate

@Nigusse
Copy link
Contributor Author

Nigusse commented Apr 30, 2020

Added new report variable Zone Air Terminal Outdoor Air Volume Flow Rate for AirTerminal:SingleDuct:ConstantVolume:Reheat object. The plot below shows the OA Vol flow rate for this air terminal.

ZoneAirTerminalOAVolumeFlowRateReheat

@Nigusse
Copy link
Contributor Author

Nigusse commented May 1, 2020

The *.rdd and *.Audit output files diffs are caused by addition of new report variable Zone Air Terminal Outdoor Air Volume Flow Rate to AirTerminal:SingleDuct:ConstantVolume:Reheat object.

\paragraph{Zone Air Terminal Outdoor Air Volume Flow Rate {[}m3/s{]}}\label{zone-air-terminal-outdoor-air-volume-flow-rate-m3s}

This output is the amount of outdoor air entering the zone. This is the average value over the frequency being reported. The amount of outdoor air is defined as the terminal unit air volume flow rate multiplied by the fraction of outdoor air entering the air loop's outside air system.

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 is description of the new report variable added for AirTerminal:SingleDuct:ConstantVolume:Reheat air terminal unit.

\subsubsection{Outputs}

There are no outputs for the constant volume reheat air terminal.

\subsection{AirTerminal:SingleDuct:ConstantVolume:NoReheat}\label{airterminalsingleductconstantvolumenoreheat}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed since a new report variable listed above is added.


\paragraph{Zone Air Terminal Outdoor Air Volume Flow Rate {[}m3/s{]}}\label{zone-air-terminal-outdoor-air-volume-flow-rate-m3s}

This output is the amount of outdoor air entering the zone. This is the average value over the frequency being reported. The amount of outdoor air is defined as the terminal unit air volume flow rate multiplied by the fraction of outdoor air entering the air loop's outside air system.

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 is missing description of existing report variable for AirTerminal:SingleDuct:ConstantVolume:NoReheat air terminal unit.

@@ -1529,7 +1529,6 @@ namespace DualDuct {
this->dd_airterminalOutlet.AirMassFlowRateMaxAvail = MassFlow;
this->dd_airterminalOutlet.AirMassFlowRateMinAvail = this->ZoneMinAirFrac * this->dd_airterminalHotAirInlet.AirMassFlowRateMax;
this->dd_airterminalOutlet.AirEnthalpy = Enthalpy;
this->OutdoorAirFlowRate = MassFlow * AirLoopOAFrac;
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 is moved to a new common member function CalcOutdoorIAirVolumeFlowRate() added for AirTerminalDualDuct:VAV and AirTerminalDualDuct:ConstantVolume objects.

// do nothing for now
}
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the new member common function added for use with AirTerminal:DualDuct:VAV and AirTerminal:DualDuct:ConstantVolume objects.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Will there ever be anything in the ELSE block here?

"System",
"Average",
sd_airterminal(SysNum).SysName);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

new report variable added for AirTerminal:SingleDuct:ConstantVolume:Reheat object.

Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

@@ -3688,9 +3696,6 @@ namespace SingleDuct {
}
}

// set OA report variable
this->OutdoorAirFlowRate = (MassFlow / StdRhoAir) * AirLoopOAFrac;

// push the flow rate history
Copy link
Contributor Author

Choose a reason for hiding this comment

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

moved to a common new member function added to singleduct air terminal struct.

// do nothing for now
}
}

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 is the common new member function added to singleduct air terminal struct. This is used for all single air terminal but currently only four of the single duct air terminals have this report variable.

Copy link
Collaborator

Choose a reason for hiding this comment

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

"Same" function in two locations. I'm assuming there's not a parent component. Should there (eventually) be one?

@@ -5310,6 +5315,8 @@ namespace SingleDuct {
if (Contaminant.GenericContamSimulation) {
Node(OutletNode).GenContam = Node(InletNode).GenContam;
}
// set OA volume flow rate report variable
this->CalcOutdoorAirVolumeFlowRate();
Copy link
Contributor Author

@Nigusse Nigusse May 1, 2020

Choose a reason for hiding this comment

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

this new member function is called during update() function call as it is used to update a report variable. This function is now invoked during update only improves performance by tiny amount.

@Nigusse
Copy link
Contributor Author

Nigusse commented May 1, 2020

This pull request is ready for review.

Copy link
Collaborator

@mitchute mitchute left a comment

Choose a reason for hiding this comment

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

Pretty straight forward changes here. Assuming CI is happy, this should go in soon.

// do nothing for now
}
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

Will there ever be anything in the ELSE block here?

"System",
"Average",
sd_airterminal(SysNum).SysName);

Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

// do nothing for now
}
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

"Same" function in two locations. I'm assuming there's not a parent component. Should there (eventually) be one?

// check AT air mass flow rates
EXPECT_EQ(expectedMassFlow, thisAirTerminalInlet.AirMassFlowRate);
EXPECT_EQ(expectedMassFlow, thisAirTerminalOutlet.AirMassFlowRate);
EXPECT_EQ(expected_OAVolFlowRate, thisAirTerminal.OutdoorAirFlowRate); // OA volume flow rate
Copy link
Collaborator

Choose a reason for hiding this comment

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

👌

HVAC,Average,Zone Air Terminal Outdoor Air Volume Flow Rate {[}m3/s{]}
\end{itemize}

\paragraph{Zone Air Terminal Outdoor Air Volume Flow Rate {[}m3/s{]}}\label{zone-air-terminal-outdoor-air-volume-flow-rate-m3s}
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

HVAC,Average,Zone Air Terminal Outdoor Air Volume Flow Rate {[}m3/s{]}
\end{itemize}

\paragraph{Zone Air Terminal Outdoor Air Volume Flow Rate {[}m3/s{]}}\label{zone-air-terminal-outdoor-air-volume-flow-rate-m3s}
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

@mitchute
Copy link
Collaborator

Looks like this fell off my radar. I'm assuming CI is still happy, so this should be able to merge soon once that has run again with the latest develop pulled in.

@Myoldmopar
Copy link
Member

Myoldmopar commented May 22, 2020

These CI results seem pretty reasonable. Added report variable showing AUD and RDD diffs. Github is currently dealing with an incident causing the gh-pages sites to not build, so that's why CI results aren't available (technically they are, and for that branch they are here).

image

I think I'll go ahead and merge this in once the unit and integration tests finish.

@mitchute
Copy link
Collaborator

mitchute commented Jun 2, 2020

One. More. Time. Seriously, though. It's gonna happen this time.

@mitchute
Copy link
Collaborator

mitchute commented Jun 2, 2020

Some minor updates were needed in the unit tests due to the recent GIO changes. I ran the unit tests and 40-ish files locally, and it all looks good. The minor AUD and RDD diffs are expected, and CI has been consistently good here for a long time. I'm not waiting for CI to run on this again. Merging.

@mitchute mitchute merged commit 6c9ad36 into develop Jun 2, 2020
@mitchute mitchute deleted the 172581983_Issue7950 branch June 2, 2020 19:38
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Defect Includes code to repair a defect in EnergyPlus
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AirTerminal:SingleDuct:ConstantVolume:NoReheat Zone Air Terminal Outdoor Air Volume Flow Rate always zero
8 participants