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

Coil:Cooling:DX updates #8558

Merged
merged 176 commits into from
Mar 26, 2021
Merged

Conversation

mjwitte
Copy link
Contributor

@mjwitte mjwitte commented Feb 22, 2021

Pull request overview

This supersedes #8292. This is a group effort by @mjwitte @Myoldmopar @rraustad and @mitchute .

Fill in feature gaps and clean up Coil:Cooling:DX, including:

  • Standard ratings
  • Additional output variables
  • Doc updates
  • Unitary system setpoint control
  • Coil selection reporting
  • Single mode and 100% OA mode
  • Latent degradation
  • Fix basin heater operating schedule name input

Still needs work:

  • User vs std ratings (from Add Report of SEER calculated using the user specified PLF curve and default AHRI curve and C_D #7821 )
  • Fix some failing unit tests - did something get broken, or are the test comparison values just outdated?
  • Fix 2-speed with cycling fan (and maybe others? - need to quantify this issue better).
  • Should Condenser flow rate default to autosize? (This IDD change would have to wait, but noting it here so it's not forgotten.)
  • Make sure UnitarySystem_SubcoolReheatDX changes in energy use are justified.

Diffs

As of 03 Mar 2021 (sunset in Chicagoland)
develop at a12bdf0 vs CoilBaseline has small or no diffs for:
UnitarySystem_HeatPumpAuto
UnitarySystem_SingleSpeedDX
All of the other files have big diffs.

This branch at add89b6 vs CoilBaseline has small or no diffs for:
UnitarySystem_HeatPumpAuto
UnitarySystem_SingleSpeedDX
UnitarySystem_DXCoilSystemAuto
UnitarySystem_FurnaceWithDXSystemRHcontrol
UnitarySystem_MultiSpeedDX
All of the other files still have big diffs.

So, this is progress.

Comparing annual simulations for this branch vs develop (new coil vs new coil) today, 26 Mar 2021
Most files have annual energy diffs on the order of 5% or less.
Files with larger diffs:

UnitarySystem_FurnaceWithDXSystemRHcontrol: +5% cooling elec, +20% heating gas, -44% Time Not Comfortable Based on Simple ASHRAE 55-2004 - but now it agrees with the baseline model - so OK.

UnitarySystem_DXCoilSystemAuto: +100% cooling electricity - wasn't even working before - not it agrees with baseline - OK.

UnitarySystem_MultiSpeedDX: +10% cooling elec, agrees with baseline now - OK.

UnitarySystem_MultiSpeedCoils_SingleMode: +25% cooling elec, still doesn't agree with baseline, so will need more work. Not holding this up for that.

UnitarySystem_SubcoolReheatDX: +25% cooling elec, no baseline to compare against, but the eso has small diffs, only the energy use has big diffs.
Biggest changes in HVAC elec are in Jan and Jul. Needs more investigation.

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 - see diff comments above
  • 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

mitchute and others added 30 commits February 25, 2020 08:43
bool SubcoolReheatFlag = false; // Subcool reheat coil control

CoilCoolingDXCurveFitSpeed &normModeNomSpeed();
CoilCoolingDXCurveFitSpeed &altModeNomSpeed();
Copy link
Contributor

Choose a reason for hiding this comment

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

This is beyond me. And also not in develop. Maybe it will make sense later.

CoilCoolingDXCurveFitSpeed & CoilCoolingDX::normModeNomSpeed()
{
return this->performance.normalMode.speeds[this->performance.normalMode.nominalSpeedIndex];
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, these are functions. This syntax is just confusing to me. See what you learn reviewing new code.

This:
this->normModeNomSpeed().condenser_air_flow_rate;

is the same as:
this->performance.normalMode.speeds[this->performance.normalMode.nominalSpeedIndex].condenser_air_flow_rate;

} else {
return this->normModeNomSpeed().condenser_air_flow_rate;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This is volume flow rate, at least I didn't see it converted to mass, and the function name is condMassFlowRate.

Real64 const coolingRate = dummyEvapInlet.MassFlowRate * (dummyEvapInlet.Enthalpy - dummyEvapOutlet.Enthalpy);
Real64 const thisMinAirHumRat = min(dummyEvapInlet.HumRat, dummyEvapOutlet.HumRat);
Real64 const sensCoolingRate = dummyEvapInlet.MassFlowRate * (Psychrometrics::PsyHFnTdbW(dummyEvapInlet.Temp, thisMinAirHumRat) -
Psychrometrics::PsyHFnTdbW(dummyEvapOutlet.Temp, thisMinAirHumRat));
Copy link
Contributor

Choose a reason for hiding this comment

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

Didn't we have a different calculation for sens cap? A new @Nigusse function somewhere?

@@ -11467,6 +11532,22 @@ namespace UnitarySystems {
DXCoils::SimDXCoilMultiMode(
state, CompName, state.dataUnitarySystems->On, FirstHVACIteration, PartLoadFrac, DehumidMode, this->m_CoolingCoilIndex, FanOpMode);
this->m_CompPartLoadRatio = PartLoadFrac;
} else if (CoilType_Num == DataHVACGlobals::CoilDX_Cooling) { // CoilCoolingDX
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, the new coil model isn't included in SP control. I thought I added that a while back.

autosize, !- Minimum Outdoor Air Flow Rate {m3/s}
autosize, !- Maximum Outdoor Air Flow Rate {m3/s}
0.0, !- Minimum Outdoor Air Flow Rate {m3/s}
0.0, !- Maximum Outdoor Air Flow Rate {m3/s}
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume you are trying to target another configuration that we don't have yet with these type of changes to idfs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, these changes were supposed to remain local. Will revert this and a couple of others.

EXPECT_NEAR(thisSys->m_CycRatio, 1.000, 0.001);
// EXPECT_NEAR(thisSys->m_SpeedRatio, 0.228062, 0.001);
EXPECT_NEAR(thisSys->m_SpeedRatio, 0.12, 0.001);
EXPECT_NEAR(sensOut, -11998.0, 210.0);
Copy link
Contributor

Choose a reason for hiding this comment

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

OK, made it through to the end. This is a lot to take in but nothing else jumped out at me.

@mjwitte
Copy link
Contributor Author

mjwitte commented Mar 18, 2021

@Myoldmopar @mitchute This is as far as I'll get for now, maybe more next week if it's still open. This isn't perfect yet, but it's good progress. Someone should look at the annual energy diffs on these files to make sure this doesn't break them.

@mitchute
Copy link
Collaborator

@mjwitte can you remind me if we have a 1:1 list of baseline vs new coil object IDF files? This was (almost) an entire pandemic ago after all... 🙃 I'll plot some of the results from these files and compare the results.

@mjwitte
Copy link
Contributor Author

mjwitte commented Mar 18, 2021

@mitchute See the list of diffs at the top of #8292 - all of the 1:1 files are listed there.

@mitchute
Copy link
Collaborator

Ah, OK. Thanks for the reminder.

@mjwitte
Copy link
Contributor Author

mjwitte commented Mar 26, 2021

Not waiting any longer on CI, thank @Myoldmopar for a local quick regression test. Merging. See top comment for discussion of diffs.

@mjwitte mjwitte merged commit ca1d335 into NREL:develop Mar 26, 2021
@mjwitte mjwitte mentioned this pull request Apr 8, 2021
20 tasks
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.

9 participants