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

Add auto-sizing capability for Nominal Air Flow Rate and Nominal Air Face Velocity in Desiccant BalancedFlow Performance DataType1 object #5743

Merged
merged 14 commits into from Jul 14, 2016

Conversation

Nigusse
Copy link
Contributor

@Nigusse Nigusse commented Jun 22, 2016

Addressed #5465 (and #5385)

Pull request overview

Added auto-sizing capability for Nominal Air Flow Rate and Nominal Air Face Velocity in Desiccant BalancedFlow Performance DataType1 object. Also consolidated Heat Recovery sizing into ReportSzingingManager routine.

Work Checklist

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)
  • At least one of the following appropriate labels must be added to this PR to be consumed into the changelog:
    • Defect: This pull request repairs a github defect issue. The github issue should be referenced in the PR description

Review Checklist

This will not be exhaustively relevant to every PR.

  • Code style (parentheses padding, variable names)
  • Functional code review (it has to work!)
  • If defect, results of running current develop vs this branch should exhibit the fix
  • CI status: all green or justified
  • Performance: CI Linux results include performance check -- verify this
  • Unit Test(s)
  • C++ checks:
    • Argument types
    • If any virtual classes, ensure virtual destructor included, other things
  • IDD changes:
    • Verify naming conventions and styles, memos and notes and defaults

@Nigusse Nigusse added the Defect Includes code to repair a defect in EnergyPlus label Jun 22, 2016
@Nigusse
Copy link
Contributor Author

Nigusse commented Jun 22, 2016

@Myoldmopar @mjwitte One of the builds for this branch failed. Here is the error message. What is causing this problem?

@nrel-bot-2
Copy link

@Nigusse @lgentile it has been 14 days since this pull request was last updated.

@rraustad
Copy link
Contributor

I don't understand the warnings in the last CI run. Those variables are used? Do they need to be explicit using DataHVACGlobals::SmallAirVolFlow, etc. ?

@@ -1442,161 +1464,111 @@ namespace HeatRecovery {
using DataHVACGlobals::Heating;
Copy link
Contributor

Choose a reason for hiding this comment

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

@rraustad Those warnings typically mean there's a stray using statement that isn't needed. Looks like these may be the culprit here since blocks of code were moved out?

@Nigusse
Copy link
Contributor Author

Nigusse commented Jul 13, 2016

@mjwitte @rraustad Looking at the code where the code chunk removed, these five variables are not needed. I was not sure what the reason was for the warning message until I saw Mike's comment.

@rraustad
Copy link
Contributor

That makes perfect sense. 5 unused variables, 5 warnings.

@rraustad
Copy link
Contributor

Review results of CI. Changes to eio file is what is expected when changing local model sizing to instead use the RequestSizing function. New inputs for autosizing appear to be working correctly. Other warnings in err file are due to other branches (surfaces < 6 and surfaces < 3 use reciprocity). Warnings reported from CI have been eliminated. All checks are green and have passed.

@rraustad rraustad merged commit 301c921 into develop Jul 14, 2016
@rraustad rraustad deleted the 5465_AutoSizeDBFPerformanceDataType1 branch July 14, 2016 13:33
@rraustad
Copy link
Contributor

I commented without closing since I still needed to merge, then merged, now I can't close this PR.

@rraustad
Copy link
Contributor

This PR is in the closed section so it must have automatically closed when I merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in 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.

None yet

6 participants