Skip to content

ZoneControl:Thermostat - Move field N1 above extensible fields. - #11822

Merged
mitchute merged 9 commits into
developfrom
7484-Tstat-extensible-fields
Oct 21, 2026
Merged

ZoneControl:Thermostat - Move field N1 above extensible fields.#11822
mitchute merged 9 commits into
developfrom
7484-Tstat-extensible-fields

Conversation

@rraustad

@rraustad rraustad commented Sep 28, 2026

Copy link
Copy Markdown
Collaborator

Pull request overview

Description of the purpose of this PR

New field added to end of ZoneControl:Thermosat causes issues with front end GUIs.

Pull Request Author

  • 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
  • If adding/removing any output files (e.g., eplustbl.*)
    • Update ..\scripts\Epl-run.bat
    • Update ..\scripts\RunEPlus.bat
    • Update ..\src\EPLaunch\ MainModule.bas, epl-ui.frm, and epl.vbp (VersionComments)
    • Update ...github\workflows\energyplus.py

Reviewer

  • 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 self-assigned this Sep 28, 2026
@rraustad rraustad added the Defect Includes code to repair a defect in EnergyPlus label Sep 28, 2026
@rraustad rraustad changed the title Move field N1 above extensible fields. ZoneControl:Thermostat - Move field N1 above extensible fields. Sep 28, 2026
@rraustad rraustad added the IDDChange Code changes impact the IDD file (cannot be merged after IO freeze) label Sep 29, 2026
@rraustad
rraustad marked this pull request as draft September 29, 2026 02:52
rraustad
rraustad commented Oct 20, 2026
View reviewed changes
Comment thread src/Transition/CreateNewIDFUsingRulesV27_1_0.f90
CurArgs = CurArgs + 1
ENDIF
nodiff=.false.

@rraustad rraustad Oct 20, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Decided to just leave the blank lines and make this robust.

Image

rraustad
rraustad commented Oct 20, 2026
View reviewed changes
Comment thread testfiles/_CrashLights_9680.idf
ThermostatSetpoint:DualSetpoint, !- Control 1 Object Type
Thermostat Setpoint Dual Setpoint 1, !- Control 1 Name
, !- Control 2 Object Type
, !- Control 2 Name
, !- Control 3 Object Type
, !- Control 3 Name
, !- Control 4 Object Type
, !- Control 4 Name
0; !- Temperature Difference Between Cutout And Setpoint {deltaC}
; !- Control 4 Name

@rraustad rraustad Oct 20, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Here's a last field that was moved and looks correct.

@github-actions

github-actions Bot commented Oct 20, 2026

Copy link
Copy Markdown

⚠️ Regressions detected on ubuntu-24.04 for commit 7aeb71f

Regression Summary
  • IDF: 5

@github-actions

github-actions Bot commented Oct 20, 2026

Copy link
Copy Markdown

⚠️ Regressions detected on macos-15 for commit 7aeb71f

Regression Summary
  • IDF: 5

@mitchute mitchute added the Staged By adding the "Staged" label, some expensive checks needed before merge are run. label Oct 20, 2026
@mitchute

mitchute commented Oct 20, 2026

Copy link
Copy Markdown
Collaborator

@rraustad. The regression results here look good.

rraustad
rraustad commented Oct 20, 2026
View reviewed changes
Comment thread src/Transition/CreateNewIDFUsingRulesV27_1_0.f90 Outdated
INTEGER :: AlphaNumI
REAL :: SaveNumber

CHARACTER(len=10) :: ZoneControlInputField='Thermostat'

@rraustad rraustad Oct 20, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Need to clean up these new vars (lines 81 and 124) that aren't needed now.

@mitchute

mitchute commented Oct 20, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

@rraustad should we add a note to src/Transition/InputRulesFiles/Rules26-2-0-to-27-1-0.md ?

@rraustad

rraustad commented Oct 20, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

ToDo:

  • Rules26-2-0-to-27-1-0.md
  • Cleanup in CreateNewIDFUsingRulesV27_1_0.f90
  • ??

@rraustad

rraustad commented Oct 20, 2026

Copy link
Copy Markdown
Collaborator Author

Interesting that the slab and basement files are the only ones throwing the idf diffs in 5 files when clearly there were hundreds of idfs that changed.

@mitchute

mitchute commented Oct 20, 2026

Copy link
Copy Markdown
Collaborator

Interesting that the slab and basement files are the only ones throwing the idf diffs in 5 files when clearly there were hundreds of idfs that changed.

Yeah, I guess I sort of glanced past that point... let me think about that.

@rraustad
rraustad marked this pull request as ready for review October 20, 2026 19:45
@github-actions

github-actions Bot commented Oct 20, 2026

Copy link
Copy Markdown

⚠️ Regressions detected on macos-15 for commit d0f220f

Regression Summary
  • IDF: 5

@github-actions

github-actions Bot commented Oct 20, 2026

Copy link
Copy Markdown

⚠️ Regressions detected on ubuntu-24.04 for commit d0f220f

Regression Summary
  • IDF: 5

@github-actions

github-actions Bot commented Oct 20, 2026

Copy link
Copy Markdown

⚠️ Regressions detected on macos-15 for commit c51c734

Regression Summary
  • IDF: 5

@github-actions

github-actions Bot commented Oct 20, 2026

Copy link
Copy Markdown

⚠️ Regressions detected on ubuntu-24.04 for commit c51c734

Regression Summary
  • IDF: 5

@mitchute

mitchute commented Oct 20, 2026

Copy link
Copy Markdown
Collaborator

Interesting that the slab and basement files are the only ones throwing the idf diffs in 5 files when clearly there were hundreds of idfs that changed.

Yeah, I guess I sort of glanced past that point... let me think about that.

It's because for slab and basement tests, we copy the expanded in.idf from the after pre-processor runs over to the run directory.

EnergyPlus/cmake/RunSimulation.cmake

Lines 100 to 101 in 8ccd723

execute_process(COMMAND ${CMAKE_COMMAND} -E copy "${IDF_PATH}" "${OUTPUT_DIR_PATH}/in.idf")
execute_process(COMMAND ${CMAKE_COMMAND} -E copy "${EPW_PATH}" "${OUTPUT_DIR_PATH}/in.epw")

For regular tests, we provide the path to the branch's IDF files that are sourced in the repo.

EnergyPlus/cmake/RunSimulation.cmake

Line 21 in 8ccd723

set(IDF_PATH "${SOURCE_DIR}/${TEST_FILE_FOLDER}/${IDF_FILE}")

EnergyPlus/cmake/RunSimulation.cmake

Line 230 in 8ccd723

COMMAND "${ENERGYPLUS_EXE}" -w "${EPW_PATH}" -d "${OUTPUT_DIR_PATH}" ${ENERGYPLUS_FLAGS_LIST} "${IDF_PATH}"

This makes sense to me.

@mitchute

mitchute commented Oct 20, 2026

Copy link
Copy Markdown
Collaborator

Thanks @rraustad. I believe this one is likely ready.

@github-actions

github-actions Bot commented Oct 20, 2026

Copy link
Copy Markdown

⚠️ Regressions detected on macos-15 for commit 911cfaf

Regression Summary
  • IDF: 5

@github-actions

github-actions Bot commented Oct 20, 2026

Copy link
Copy Markdown

⚠️ Regressions detected on ubuntu-24.04 for commit 911cfaf

Regression Summary
  • IDF: 5

@mitchute

mitchute commented Oct 21, 2026

Copy link
Copy Markdown
Collaborator

Thanks @rraustad. Merging.

@mitchute
mitchute merged commit c912ec8 into develop Oct 21, 2026
11 checks passed
@mitchute
mitchute deleted the 7484-Tstat-extensible-fields branch October 21, 2026 01:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mitchute mitchute mitchute approved these changes

Assignees

@rraustad rraustad

Labels

Defect Includes code to repair a defect in EnergyPlus IDDChange Code changes impact the IDD file (cannot be merged after IO freeze) Staged By adding the "Staged" label, some expensive checks needed before merge are run.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

ZoneControl:Thermostat should move the extensible fields at the end

3 participants

@rraustad @mitchute @nrel-bot-2c