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 autosize to Chiller:Absorption:Indirect flow rate fields #5361

Merged
merged 5 commits into from Dec 15, 2015

Conversation

Projects
None yet
7 participants
@JasonGlazer
Copy link
Contributor

commented Dec 9, 2015

Addresses #4406

@nrel-bot-2

This comment has been minimized.

Copy link

commented on b684279 Dec 9, 2015

109806134-AbsChlrBlankWaterFlow (JasonGlazer) - x86_64-Linux-Ubuntu-14.04-gcc-4.8: OK (1983 of 1983 tests passed)

Build Badge Test Badge

This comment has been minimized.

Copy link

replied Dec 9, 2015

109806134-AbsChlrBlankWaterFlow (JasonGlazer) - x86_64-Linux-Ubuntu-14.04-cppcheck-1.61: OK (0 of 0 tests passed)

Build Badge

This comment has been minimized.

Copy link

replied Dec 9, 2015

109806134-AbsChlrBlankWaterFlow (JasonGlazer) - x86_64-MacOS-10.9-clang: OK (1977 of 1977 tests passed)

Build Badge Test Badge

This comment has been minimized.

Copy link

replied Dec 9, 2015

109806134-AbsChlrBlankWaterFlow (JasonGlazer) - x86_64-Linux-Ubuntu-14.04-gcc-4.8-UnitTestsCoverage-Debug: OK (909 of 909 tests passed)

Build Badge Test Badge Coverage Badge

This comment has been minimized.

Copy link

replied Dec 9, 2015

109806134-AbsChlrBlankWaterFlow (JasonGlazer) - Win64-Windows-7-VisualStudio-12: OK (1983 of 1983 tests passed)

Build Badge Test Badge

This comment has been minimized.

Copy link

replied Dec 9, 2015

109806134-AbsChlrBlankWaterFlow (JasonGlazer) - x86_64-Linux-Ubuntu-14.04-gcc-4.8-IntegrationCoverage-Debug: OK (1447 of 1447 tests passed)

Build Badge Test Badge Coverage Badge

This comment has been minimized.

Copy link

replied Dec 9, 2015

109806134-AbsChlrBlankWaterFlow (JasonGlazer) - i386-Windows-7-VisualStudio-12: OK (1983 of 1983 tests passed)

Build Badge Test Badge

@JasonGlazer JasonGlazer self-assigned this Dec 11, 2015

@JasonGlazer

This comment has been minimized.

Copy link
Contributor Author

commented Dec 11, 2015

@Myoldmopar @rraustad @mjwitte Could one of you please take a look at the code and merge in the change?

@Myoldmopar

This comment has been minimized.

Copy link
Member

commented Dec 14, 2015

Hmm. I would have expected to see a \default autosize in there rather than have the code make it quietly change a zero into an autosize. Curious what @mjwitte thinks.

@mjwitte

This comment has been minimized.

Copy link
Contributor

commented Dec 14, 2015

Agreed. \default autosize and possibly not required-field - does it even make sense for any field with a default to be required?

I went looking for examples of other similar fields in the IDD and I find several flavors,

  • others like this one currently is (not required, no default),
  • some are required, but no default
  • some are required and have default autosize
  • some are not required but have default autosize

This issue should prompt us to agree on and document a standard policy for autosizable fields. Then fix all of them to be consistent. @kbenne @macumber Any thoughts?

@JasonGlazer

This comment has been minimized.

Copy link
Contributor Author

commented Dec 15, 2015

I will change it to \default autosize and remove the \required-field.

@nrel-bot-2

This comment has been minimized.

Copy link

commented on f526a94 Dec 15, 2015

109806134-AbsChlrBlankWaterFlow (JasonGlazer) - x86_64-Linux-Ubuntu-14.04-cppcheck-1.61: OK (0 of 0 tests passed)

Build Badge

This comment has been minimized.

Copy link

replied Dec 15, 2015

109806134-AbsChlrBlankWaterFlow (JasonGlazer) - x86_64-MacOS-10.9-clang: OK (1979 of 1979 tests passed)

Build Badge Test Badge

This comment has been minimized.

Copy link

replied Dec 15, 2015

109806134-AbsChlrBlankWaterFlow (JasonGlazer) - x86_64-Linux-Ubuntu-14.04-gcc-4.8: OK (1985 of 1985 tests passed)

Build Badge Test Badge

This comment has been minimized.

Copy link

replied Dec 15, 2015

109806134-AbsChlrBlankWaterFlow (JasonGlazer) - x86_64-Linux-Ubuntu-14.04-gcc-4.8-UnitTestsCoverage-Debug: OK (911 of 911 tests passed)

Build Badge Test Badge Coverage Badge

This comment has been minimized.

Copy link

replied Dec 15, 2015

109806134-AbsChlrBlankWaterFlow (JasonGlazer) - i386-Windows-7-VisualStudio-12: OK (1985 of 1985 tests passed)

Build Badge Test Badge

This comment has been minimized.

Copy link

replied Dec 15, 2015

109806134-AbsChlrBlankWaterFlow (JasonGlazer) - x86_64-Linux-Ubuntu-14.04-gcc-4.8-IntegrationCoverage-Debug: OK (1449 of 1449 tests passed)

Build Badge Test Badge Coverage Badge

This comment has been minimized.

Copy link

replied Dec 15, 2015

109806134-AbsChlrBlankWaterFlow (JasonGlazer) - Win64-Windows-7-VisualStudio-12: OK (1985 of 1985 tests passed)

Build Badge Test Badge

@@ -516,15 +516,25 @@ namespace ChillerIndirectAbsorption {
IndirectAbsorber( AbsorberNum ).TempDesCondIn = rNumericArgs( 6 );
IndirectAbsorber( AbsorberNum ).MinCondInletTemp = rNumericArgs( 7 );
IndirectAbsorber( AbsorberNum ).TempLowLimitEvapOut = rNumericArgs( 8 );
IndirectAbsorber( AbsorberNum ).EvapVolFlowRate = rNumericArgs( 9 );
if ( IndirectAbsorber( AbsorberNum ).EvapVolFlowRate == AutoSize ) {
if ( rNumericArgs( 9 ) != 0.0 ){

This comment has been minimized.

Copy link
@Myoldmopar

Myoldmopar Dec 15, 2015

Member

So now that we have \default autosize, if it is blank, it will be changed to AutoSize by the input processor. Does that mean we can simplify this down a little? It will never be zero now. It will be either:

  • blank which means -99999
  • "AutoSize" which means -99999, or
  • greater than zero.

So this block:

if ( rNumericArgs( 9 ) != 0.0 ){
  IndirectAbsorber( AbsorberNum ).EvapVolFlowRate = rNumericArgs( 9 );
    if ( IndirectAbsorber( AbsorberNum ).EvapVolFlowRate == AutoSize ) {
        IndirectAbsorber( AbsorberNum ).EvapVolFlowRateWasAutoSized = true;
    }
} else {
    IndirectAbsorber( AbsorberNum ).EvapVolFlowRate = AutoSize;
        IndirectAbsorber( AbsorberNum ).EvapVolFlowRateWasAutoSized = true;
}

should just be:

IndirectAbsorber( AbsorberNum ).EvapVolFlowRate = rNumericArgs( 9 );
if ( IndirectAbsorber( AbsorberNum ).EvapVolFlowRate == AutoSize ) {
    IndirectAbsorber( AbsorberNum ).EvapVolFlowRateWasAutoSized = true;
}

This comment has been minimized.

Copy link
@JasonGlazer

JasonGlazer Dec 15, 2015

Author Contributor

Ok, I will revert that.

Myoldmopar added a commit that referenced this pull request Dec 15, 2015

@Myoldmopar Myoldmopar merged commit 664a18c into develop Dec 15, 2015

@Myoldmopar Myoldmopar deleted the 109806134-AbsChlrBlankWaterFlow branch Dec 15, 2015

@nrel-bot-3

This comment has been minimized.

Copy link

commented on 082b0e4 Dec 15, 2015

109806134-AbsChlrBlankWaterFlow (JasonGlazer) - x86_64-MacOS-10.9-clang: OK (0 of 0 tests passed)

Build Badge Test Badge

This comment has been minimized.

Copy link

replied Dec 15, 2015

109806134-AbsChlrBlankWaterFlow (JasonGlazer) - i386-Windows-7-VisualStudio-12: OK (0 of 0 tests passed)

Build Badge Test Badge

This comment has been minimized.

Copy link

replied Dec 15, 2015

109806134-AbsChlrBlankWaterFlow (JasonGlazer) - Win64-Windows-7-VisualStudio-12: OK (0 of 0 tests passed)

Build Badge Test Badge

@Myoldmopar Myoldmopar added the Defect label Dec 16, 2015

@JasonGlazer JasonGlazer restored the 109806134-AbsChlrBlankWaterFlow branch Jan 29, 2016

@JasonGlazer JasonGlazer deleted the 109806134-AbsChlrBlankWaterFlow branch Jan 29, 2016

@Myoldmopar

This comment has been minimized.

Copy link
Member

commented Feb 16, 2016

@JasonGlazer Can you change the title of this PR to a more user-digestable form? Thanks!

@JasonGlazer JasonGlazer changed the title 109806134 abs chlr blank water flow Add autosize to Chiller:Absorption:Indirect flow rate fields Feb 16, 2016

@macumber

This comment has been minimized.

Copy link
Contributor

commented Feb 22, 2016

I've never understood fields that have a default and are required, when OpenStudio parses that it issues a warning and makes the field not required.

@mjwitte

This comment has been minimized.

Copy link
Contributor

commented Feb 22, 2016

@macumber That's a valid point. Please post a new issue and attach a list of those warnings, if that's convenient.

@mjwitte mjwitte referenced this pull request Feb 23, 2016

Closed

IDD Cleanup post v8.5 #5502

1 of 1 task complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.