Skip to content

Remove fmt - Part 2: Remove {R} Files A-D#11588

Draft
mitchute wants to merge 2 commits into
developfrom
remove-fmt-part-2
Draft

Remove fmt - Part 2: Remove {R} Files A-D#11588
mitchute wants to merge 2 commits into
developfrom
remove-fmt-part-2

Conversation

@mitchute
Copy link
Copy Markdown
Collaborator

@mitchute mitchute commented May 11, 2026

Pull request overview

Continues the process of removing third-party/fmt library in lieu of std::format for files A-D.

Description of the purpose of this PR

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

@mitchute mitchute self-assigned this May 11, 2026
@mitchute mitchute added the Refactoring Includes code changes that don't change the functionality of the program, just perform refactoring label May 11, 2026
@mitchute mitchute marked this pull request as draft May 12, 2026 00:10
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 12, 2026

⚠️ Regressions detected on ubuntu-24.04 for commit 589f87c

Regression Summary
  • EIO: 450
  • Table Small Diffs: 300
  • Table Big Diffs: 54
  • ERR: 27
  • DFS: 1

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 12, 2026

⚠️ Regressions detected on macos-14 for commit 589f87c

Regression Summary
  • EIO: 449
  • Table Small Diffs: 300
  • Table Big Diffs: 54
  • ERR: 27
  • DFS: 1

rraustad
rraustad reviewed May 12, 2026
View reviewed changes
Comment thread src/EnergyPlus/ConvectionCoefficients.cc

static constexpr std::string_view Format_901(
"Surface Convection Parameters,{},{},{:.2R},{:.2R},{:.2R},{},{:.2R},{:.2R},{:.2R},{:.2R},{},{},{}\n");
"Surface Convection Parameters,{},{},{:.2f},{:.2f},{:.2f},{},{:.2f},{:.2f},{:.2f},{:.3G},{},{},{}\n");
Copy link
Copy Markdown
Collaborator

@rraustad rraustad May 12, 2026

Choose a reason for hiding this comment

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

The .3G caught my eye. Looks OK with better precision.

Image

rraustad
rraustad reviewed May 12, 2026
View reviewed changes
Comment thread src/EnergyPlus/Autosizing/Base.cc Outdated
static constexpr std::string_view Format_990(
"! <Component Sizing Information>, Component Type, Component Name, Input Field Description, Value\n");
static constexpr std::string_view Format_991(" Component Sizing Information, {}, {}, {}, {:.5R}\n");
static constexpr std::string_view Format_991(" Component Sizing Information, {}, {}, {}, {:.5f}\n");
Copy link
Copy Markdown
Collaborator

@rraustad rraustad May 12, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

I am assuming this change is the reason for this type of diff. Maybe use E here?
File: UnitarySystem_ZoneVSWSHP_wDOAS

Image

shorowit
shorowit reviewed May 12, 2026
View reviewed changes
Comment thread tst/EnergyPlus/unit/Autosizing/HeatingWaterflowSizing.unit.cc Outdated
std::string(" Component Sizing Information, Coil:Heating:Water, MyWaterCoil, Design Size Maximum Water Flow Rate [m3/s], 1.21516E-004\n"
" Component Sizing Information, Coil:Heating:Water, MyWaterCoil, User-Specified Maximum Water Flow Rate [m3/s], 2.00000E-004\n");
std::string(" Component Sizing Information, Coil:Heating:Water, MyWaterCoil, Design Size Maximum Water Flow Rate [m3/s], 0.00012\n"
" Component Sizing Information, Coil:Heating:Water, MyWaterCoil, User-Specified Maximum Water Flow Rate [m3/s], 0.00020\n");
Copy link
Copy Markdown
Contributor

@shorowit shorowit May 12, 2026

Choose a reason for hiding this comment

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

I assume these are changes from scientific notation to fixed precision? Hopefully this won't cause small buildings (e.g., residential homes) dropping off the low end and simply reporting something like 0.00000, we've had issues like that in the past. (My comment is not specific to this object/output, it's more general.)

@mitchute
Copy link
Copy Markdown
Collaborator Author

mitchute commented May 13, 2026

@rraustad @shorowit I just discovered this formatting option. It's not perfect, but IMO, it is a bit more palatable visually than pure scientific notation.

Take a look at 90012ae and let me know what you think.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 13, 2026

⚠️ Regressions detected on ubuntu-24.04 for commit 8cc7ed9

Regression Summary
  • EIO: 661
  • Table Small Diffs: 506
  • Table Big Diffs: 52
  • ERR: 27
  • DFS: 1

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 13, 2026

⚠️ Regressions detected on macos-14 for commit 8cc7ed9

Regression Summary
  • EIO: 659
  • Table Small Diffs: 506
  • Table Big Diffs: 52
  • ERR: 27
  • DFS: 1

@rraustad
Copy link
Copy Markdown
Collaborator

rraustad commented May 14, 2026

@rraustad @shorowit I just discovered this formatting option. It's not perfect, but IMO, it is a bit more palatable visually than pure scientific notation.

That's interesting. The smaller the number the more decimal resolution. I guess a large number would look like xxxxxxx000? I guess this is fine and could be changed later if desired.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rraustad rraustad rraustad left review comments

@shorowit shorowit shorowit left review comments

Assignees

@mitchute mitchute

Labels

Refactoring Includes code changes that don't change the functionality of the program, just perform refactoring

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@mitchute @rraustad @shorowit @nrel-bot-2c