CppCheck VentilatedSlab

[rraustad]

Pull request overview

• Fixes style issues
• Use CppCheck as guide

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]

Reducing SurfNum showed this dangling use of that local (and below at 4348). I suspect the value of SurfNum here would be the last surface in the surface list from the previous loop (old line 4487). This seems wrong so I changed it (bc it wouldn't compile otherwise) to be what I thought it should be (i.e., all surface radiation = 0). @RKStrand ?

[RKStrand]

Thanks for sending this my way to take a look at. I see now why this won't compile--not sure how it's been compiling at all before you made this change. This is definitely a bug that needs to be fixed.

The problem is that it is not as simple as getting rid of the (SurfNum). The reason that should not be done is because that array includes the source/sink inside that surface and could be affected by other systems (like low temp radiant). Zeroing them all out would basically shut down every system. What should have happened here is that the QRadSysSource array should be zeroed out only for the surfaces that are associated with this ventilated slab (which, given were these errors are happening in the code, is being turned off for no flow).

So, I think what each of these two problem lines of code need to be replaced with is:

    for (RadSurfNum = 1; RadSurfNum <= ventSlab.NumOfSurfaces; ++RadSurfNum) {
        SurfNum = ventSlab.SurfacePtr(RadSurfNum);
        state.dataHeatBalFanSys->QRadSysSource(SurfNum) = 0.0;
    }

That would then zero out the source/sink term for each of the surfaces associated with the ventilated slab without messing with any other surfaces that might be served by a different ventilated slab, radiant system, etc. that could also be setting values in the QRadSysSource array. Does this make sense/help?

[rraustad]

That makes perfect sense. I almost did that but incorrectly thought this array was specific to this ventilated slab. I will make the suggested change. BTW, this did compile before this change and zero'd only the last surface in the list. Thanks @RKStrand.

[github-actions]

⚠️ Regressions detected on macos-14 for commit 67322e0

Regression Summary

• EIO: 2
• ESO Big Diffs: 2
• MTR Big Diffs: 2
• Table Big Diffs: 2
• Table String Diffs: 2
• ERR: 1

• View Results
• Download Regressions

[Myoldmopar]

Holding off on this one while we evaluate the SurfNum and regressions.

[github-actions]

⚠️ Regressions detected on macos-14 for commit 0f8a53d

Regression Summary

• EIO: 1
• ERR: 1
• ESO Big Diffs: 1
• MTR Small Diffs: 1
• Table Big Diffs: 1

• View Results
• Download Regressions

[rraustad]

This Mac CI diffs in VentilatedSlab_SeriesSlabs are very small. I assume this is due to zeroing the radiation for all surfaces now when the slab is off instead of just the last one in the surface list. What is a little disturbing is the large number of warnings in the error file with water temps over 115 C resulting in glycol warnings.