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

Relax Conduction Transfer Function time step limit #10614

Merged
merged 1 commit into from
Jul 31, 2024

Conversation

rraustad
Copy link
Contributor

Pull request overview

  • Fixes Thick dense construction fails with CTF #10613
  • Defect file now runs successfully. The logic is if most constructions pass CTF calculations with a limit of 4 hours then all will also pass, and converge early, with a limit of 7 hours. This change simply allows more construction types to pass CTF.

NOTE: ENHANCEMENTS MUST FOLLOW A SUBMISSION PROCESS INCLUDING A FEATURE PROPOSAL AND DESIGN DOCUMENT PRIOR TO SUBMITTING CODE

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

@rraustad rraustad added the Defect Includes code to repair a defect in EnergyPlus label Jul 19, 2024
@rraustad
Copy link
Contributor Author

rraustad commented Jul 19, 2024

The eio file shows the CTF values before and after this fix have not changed. So that would mean this construction did converge but at 5 hours instead of 4 and fatal'd out. Chicago Ohare Intl Ap,IL,USA,TMY3,725300

develop:

! <Construction CTF>,Construction Name,Index,#Layers,#CTFs,Time Step {hours},ThermalConductance {w/m2-K},OuterThermalAbsorptance,InnerThermalAbsorptance,OuterSolarAbsorptance,InnerSolarAbsorptance,Roughness
! <Material CTF Summary>,Material Name,Thickness {m},Conductivity {w/m-K},Density {kg/m3},Specific Heat {J/kg-K},ThermalResistance {m2-K/w}
! <CTF>,Time,Outside,Cross,Inside,Flux (except final one)
Construction CTF,ALCAN FLAT ROOF,   1,   1,  17,   5.000,     0.7546E-01,   0.900,   0.900,   0.700,   0.700,Rough
 Material CTF Summary,ALCAN FLAT ROOF,  0.3048,         0.023,   1121.260,     1465.380,   13.25    
 CTF,  17,     -0.10420722E-11,     -0.24701322E-18,     -0.10420453E-11,      0.46687400E-12
 CTF,  16,      0.11550858E-09,     -0.12868720E-16,      0.11550861E-09,     -0.52596772E-10
 CTF,  15,     -0.73953661E-08,     -0.18623140E-16,     -0.73953661E-08,      0.34311979E-08
 CTF,  14,      0.30446007E-06,      0.10978988E-14,      0.30446007E-06,     -0.14435458E-06
 CTF,  13,     -0.85057972E-05,      0.15684485E-12,     -0.85057972E-05,      0.41350943E-05
 CTF,  12,      0.16620262E-03,      0.94004761E-11,      0.16620262E-03,     -0.83158534E-04
 CTF,  11,     -0.23126281E-02,      0.26742991E-09,     -0.23126281E-02,      0.11957452E-02
 CTF,  10,      0.23170049E-01,      0.38318642E-08,      0.23170049E-01,     -0.12432638E-01
 CTF,   9,     -0.16829768    ,      0.28542617E-07,     -0.16829768    ,      0.94112287E-01
 CTF,   8,      0.88965236    ,      0.11174684E-06,      0.88965236    ,     -0.52052509    
 CTF,   7,      -3.4257719    ,      0.22814977E-06,      -3.4257719    ,       2.1045477    
 CTF,   6,       9.5835871    ,      0.23599832E-06,       9.5835871    ,      -6.1997213    
 CTF,   5,      -19.324267    ,      0.11683747E-06,      -19.324267    ,       13.193473    
 CTF,   4,       27.641566    ,      0.25023450E-07,       27.641566    ,      -19.947902    
 CTF,   3,      -27.240039    ,      0.19343052E-08,      -27.240039    ,       20.797128    
 CTF,   2,       17.526885    ,      0.36632020E-10,       17.526885    ,      -14.160978    
 CTF,   1,      -6.6105213    ,      0.41623424E-12,      -6.6105213    ,       5.6511709    
 CTF,   0,       1.1061926    ,     -0.59578846E-13,       1.1061926    

************* EnergyPlus Terminated--Fatal Error Detected. 13 Warning; 1 Severe Errors; Elapsed Time=00hr 00min  0.16sec

this branch:

! <Construction CTF>,Construction Name,Index,#Layers,#CTFs,Time Step {hours},ThermalConductance {w/m2-K},OuterThermalAbsorptance,InnerThermalAbsorptance,OuterSolarAbsorptance,InnerSolarAbsorptance,Roughness
! <Material CTF Summary>,Material Name,Thickness {m},Conductivity {w/m-K},Density {kg/m3},Specific Heat {J/kg-K},ThermalResistance {m2-K/w}
! <CTF>,Time,Outside,Cross,Inside,Flux (except final one)
 Construction CTF,ALCAN FLAT ROOF,   1,   1,  17,   5.000,     0.7546E-01,   0.900,   0.900,   0.700,   0.700,Rough
 Material CTF Summary,ALCAN FLAT ROOF,  0.3048,         0.023,   1121.260,     1465.380,   13.25    
 CTF,  17,     -0.10420722E-11,     -0.24701322E-18,     -0.10420453E-11,      0.46687400E-12
 CTF,  16,      0.11550858E-09,     -0.12868720E-16,      0.11550861E-09,     -0.52596772E-10
 CTF,  15,     -0.73953661E-08,     -0.18623140E-16,     -0.73953661E-08,      0.34311979E-08
 CTF,  14,      0.30446007E-06,      0.10978988E-14,      0.30446007E-06,     -0.14435458E-06
 CTF,  13,     -0.85057972E-05,      0.15684485E-12,     -0.85057972E-05,      0.41350943E-05
 CTF,  12,      0.16620262E-03,      0.94004761E-11,      0.16620262E-03,     -0.83158534E-04
 CTF,  11,     -0.23126281E-02,      0.26742991E-09,     -0.23126281E-02,      0.11957452E-02
 CTF,  10,      0.23170049E-01,      0.38318642E-08,      0.23170049E-01,     -0.12432638E-01
 CTF,   9,     -0.16829768    ,      0.28542617E-07,     -0.16829768    ,      0.94112287E-01
 CTF,   8,      0.88965236    ,      0.11174684E-06,      0.88965236    ,     -0.52052509    
 CTF,   7,      -3.4257719    ,      0.22814977E-06,      -3.4257719    ,       2.1045477    
 CTF,   6,       9.5835871    ,      0.23599832E-06,       9.5835871    ,      -6.1997213    
 CTF,   5,      -19.324267    ,      0.11683747E-06,      -19.324267    ,       13.193473    
 CTF,   4,       27.641566    ,      0.25023450E-07,       27.641566    ,      -19.947902    
 CTF,   3,      -27.240039    ,      0.19343052E-08,      -27.240039    ,       20.797128    
 CTF,   2,       17.526885    ,      0.36632020E-10,       17.526885    ,      -14.160978    
 CTF,   1,      -6.6105213    ,      0.41623424E-12,      -6.6105213    ,       5.6511709    
 CTF,   0,       1.1061926    ,     -0.59578846E-13,       1.1061926    

************* EnergyPlus Completed Successfully-- 16 Warning; 0 Severe Errors; Elapsed Time=00hr 00min  3.38sec

@rraustad rraustad added this to the EnergyPlus 24.2 milestone Jul 19, 2024
@rraustad rraustad requested a review from RKStrand July 21, 2024 00:44
Copy link
Contributor

@RKStrand RKStrand left a comment

Choose a reason for hiding this comment

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

@rraustad I get that this allows the defect input file to run to completion. As the comments say here, the use of 4 hours was arbitrary so I don't have an argument against bumping this up to 7 hours. The material used in this case is basically one foot of high R-value concrete. The combination of low conductivity with relatively high specific heat, density, and thickness result in the long time step. My concern is: what do the results say? Do they actually make sense? Will the user know how to tell? It seems like a good idea to run a modified version of 1ZoneUncontrolled with this construction to see if the design day results make sense (might have to bump up the number of warmup days) and if the annual results seem reasonable. In the design day, if it can't get to steady periodic (or at least headed there), that's a potential concern. In the annual result, if the temperatures seemed to wander all over the place, that's a sign that the CTFs might not be correct. I guess the reality is that we don't have a way to test this outside of someone doing something like this and I am guessing that the documentation does not tell the user what to look for? So, I'm not opposed to this change, but I wonder if the result will be bogus simulation results. Just a suspicion on my part.

@RKStrand
Copy link
Contributor

Well, I thought I was making an "in-line" comment, but I guess I don't know what I am doing. The reference in the previous post to "the comments" was pointing to the comments that describe the constant parameter that got adjusted here.

In any case, I'm not really asking for a change to the fix. Maybe what is needed is some additional documentation? The IO Ref is not set-up for discussing something like this, and the Eng Ref is heavy on the details of how the calculation is made. There's nothing really "practical" about what to look for when the time step gets longer. Given what is there now, I'm not even sure where it would go--a new sub-section or note in the "Conduction Through the Wall" part of Eng Ref? _CTFTestsPart2.idf is a file that I used early on in development to test what was going on. Those tests were by no means exhaustive and the material chosen by the user is clearly more challenging the "slowhwconc" material that was tested in _CTFTestsPart1.idf.

Or maybe just an additional "warning" message when the time step gets above "something" that they need to look at the results careful and what to try to see what is going on? I mean, the material properties chosen here are unusual. But the warning message might end up being kind of long as it tries to describe a lot so maybe that isn't the right approach.

I think my concern is that the user will think "completed successfully" will mean that the results are good. That was actually not guaranteed before either, so this is really not a new concern.

@Myoldmopar @mjwitte Any thoughts on this?

@rraustad
Copy link
Contributor Author

All good suggestions. I like the idea of comparing this material using an original example file to see what happens.

@mjwitte mjwitte self-assigned this Jul 31, 2024
@mjwitte
Copy link
Contributor

mjwitte commented Jul 31, 2024

Discussed this on the iteration call today and agreed to merge.

@mjwitte mjwitte merged commit 0a139a5 into develop Jul 31, 2024
17 checks passed
@mjwitte mjwitte deleted the 10613-CTF-fails-with-thick-dense-material branch July 31, 2024 19:48
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.

Thick dense construction fails with CTF
7 participants