Skip to content

Fix DOASMixer primary flow problem and add error trap#6333

Merged
Myoldmopar merged 1 commit intodevelopfrom
DOASMixer6332
Sep 22, 2017
Merged

Fix DOASMixer primary flow problem and add error trap#6333
Myoldmopar merged 1 commit intodevelopfrom
DOASMixer6332

Conversation

@mjwitte
Copy link
Copy Markdown
Contributor

@mjwitte mjwitte commented Sep 20, 2017 •
edited
Loading

Pull request overview

Addresses #6332 by limiting the DOAS mixer primary air flow to be no greater than the mixed air outlet mass flow rate which is set by the zone equipment. This fixes a problem introduced by earlier DOAS mixer changes for v8.8 ( #6223 and #6252), so this doesn't need to be published.

Question to reviewers: Do you think the new fatal error is a good idea or should it just be a one-time warning? If it happens, the user probably can't do anything to fix it. Or maybe it should be a debug assert instead?

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
    • Refactoring: This pull request includes code changes that don't change the functionality of the program, just perform refactoring
    • NewFeature: This pull request includes code to add a new feature to EnergyPlus
    • Performance: This pull request includes code changes that are directed at improving the runtime performance of EnergyPlus
    • DoNoPublish: This pull request includes changes that shouldn't be included in the changelog

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

@mjwitte mjwitte added the DoNotPublish Includes changes that shouldn't be reported in the changelog label Sep 20, 2017
@mjwitte mjwitte added this to the EnergyPlus 8.8.0 milestone Sep 20, 2017
@mjwitte mjwitte mentioned this pull request Sep 21, 2017
Merged
20 tasks
@mjwitte
Copy link
Copy Markdown
Contributor Author

mjwitte commented Sep 21, 2017

Diffs as expected in DOAToUnitVentilator (variable OA flow controls with inlet side mixer) and DOAToWaterToAirHPInlet (cycling WSHP fans with inlet side mixer).

@mjwitte mjwitte requested review from Nigusse and lgu1234 September 21, 2017 13:46
@lgu1234
Copy link
Copy Markdown
Contributor

lgu1234 commented Sep 21, 2017

@mjwitte @Nigusse I tested all DOA* files. The fix worked. Then I commented a line and keep an error trap to run all DOA* files again:

	if ( this->MixerType == ATMixer_InletSide ) {

// Node( this->PriInNode ).MassFlowRate = min( Node( this->PriInNode ).MassFlowRate, Node( this->MixedAirOutNode ).MassFlowRate );
}
The following example files had a fatal error caught by the error trap:

DOAToFanCoilInlet
DOAToPTAC
DOAToUnitVentilator
DOAToWaterToAirHPInlet

because the mixed node has zero flow and the primary node has a small mass flow rate, except DOAToUnitVentilator.

I think the fatal error is good. I approve this PR.

I have a new question for future consideration. If a DOAs requires an amount of OA (mdot of Primary Node > 0), and a system is off due to no load (mdot of mixed node = 0), do we force the system to pass required OA without coil operation?

@Nigusse
Copy link
Copy Markdown
Contributor

Nigusse commented Sep 21, 2017

@lgu1234 Yes for the option the primary air system to provide OA evenif the coil is off.

@Myoldmopar Myoldmopar merged commit 325f2ba into develop Sep 22, 2017
@Myoldmopar Myoldmopar deleted the DOASMixer6332 branch September 22, 2017 19:58
@Myoldmopar Myoldmopar mentioned this pull request Sep 22, 2017
Closed
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lgu1234 lgu1234 lgu1234 approved these changes

@Nigusse Nigusse Awaiting requested review from Nigusse

Assignees

No one assigned

Labels

DoNotPublish Includes changes that shouldn't be reported in the changelog

Projects

None yet

Milestone

EnergyPlus 8.8.0

Development

Successfully merging this pull request may close these issues.

7 participants

@mjwitte @lgu1234 @Nigusse @nrel-bot @nrel-bot-2 @nrel-bot-3 @Myoldmopar