Experimental reduction in code size - don't merge - testing regressions#11472
Open
lefticus wants to merge 418 commits intoNatLabRockies:developfrom
Open
Experimental reduction in code size - don't merge - testing regressions#11472lefticus wants to merge 418 commits intoNatLabRockies:developfrom
lefticus wants to merge 418 commits intoNatLabRockies:developfrom
Conversation
Contributor
Author
|
@jmarrec I hope you all don't mind that I'm running this experiment. Check out the side-by-side diffs (note that common code is extracted and is on top) @Myoldmopar @kbenne I don't think you're currently actively working on E+, but you also might find it interesting As stated in the PR I don't expect this to get accepted. I'm sure it's going to step on other people's work and be far too risky. The changes are 100% generated by Claude. As you all know, I like using E+ as a test bed for the rules and practices I teach, since it's
I wanted to see what the current generation of tools could do given a narrowly focused task of "find biggest function, try to make it DRY" Basic rules:
More than 4 changes and it starts to forget the rules The thing I'm missing is the easy ability to run the regressions locally (I didn't feel like setting it up and taking the time per small change). Hence the PR. |
Collaborator
|
@lefticus no problem. Test away. |
Contributor
Author
Thank you very much. Impressively so far the only regression is a field order diff in RDD. and performance is effectively identical |
Collaborator
Yeah, that is impressive, and I think that would be a totally acceptable outcome for an effort such as this. |
lefticus
force-pushed
the
dry-refactor-agent
branch
2 times, most recently
from
3bb64b5 to
aa4b11f
Compare
…e reportUnitarySystem The reportUnitarySystem function had ~15 near-identical copies of the ancillary electric power/consumption calculation pattern across its cooling and heating coil switch statements. Extract this into calcAuxElecPower(). Similarly, the speed variable triplet assignment (CycRatio, SpeedRatio, SpeedNum = max of cooling/heating values) appeared ~8 times and is now setMergedSpeedVars(). Net reduction: ~91 lines. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…tarySystem The four branches (HeatingLoad + positive/negative output, CoolingLoad + positive/negative output) were pairwise identical. The HeatingLoad flag only affected behavior at the QTotUnitOut == 0.0 boundary, but all four paths converge to the same result: positive output => heating rates, non-positive output => cooling rates. Collapse to a single boolean test. Also extract isPTUnit local to avoid repeating the three-way sysType check. Net reduction: ~14 lines. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… checks The condition (m_sysType == PackagedAC || m_sysType == PackagedHP || m_sysType == PackagedWSHP) appeared ~39 times throughout UnitarySystem.cc, including both positive and negated forms. Replace all three-way checks with the new isPackagedUnit() const method for readability and maintainability. Partial two-way checks (PackagedAC || PackagedHP only) are left unchanged since they intentionally exclude PackagedWSHP. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…edCoils The four variable-speed coil types (CoolingAirToAir, HeatingAirToAir, CoolingWaterToAirVS, HeatingWaterToAirVS) all called SimVariableSpeedCoils with identical arguments except for the speed ratio (m_CoolingSpeedRatio vs m_HeatingSpeedRatio). Merge into a single call that selects the speed ratio based on whether it is a cooling or heating coil. Net reduction: ~39 lines. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… input loops The FluidTCtrl-HP and FluidTCtrl-HR condenser input loops in GetVRFInputData shared ~100 lines of nearly identical code for reading availability schedules, zone TU lists, refrigerant, rated capacity/COP, OA temperature ranges, SH/SC, IU configuration, OU fan data, quadratic curve coefficients, pipe parameters (with RefPipEquLen validation), crank case parameters, and condenser type. This consolidates those into a single readFluidCtrlCommonFields lambda with parameterized numeric field indices. Also removes a dead-code thermostat priority block (hardcoded to "LoadPriority") from the HP loop. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… MSHP sizing in sizeSystem The sizeSystem function contained 8 nearly identical blocks for initializing multi-speed flow rate arrays (resize if empty) and multiple repeated loops for auto-sizing MSHP flow ratios and computing per-speed flow rates. Four local lambdas consolidate these patterns: - initMultiSpeedFlowArrays: replaces 8 repeated resize-if-empty blocks - autoSizeFlowRatios: replaces 6 identical AutoSize default ratio loops - assignMSHPFlowRates: replaces 5 identical per-speed flow computation loops - setNoLoadFlowFromMSHP: replaces 4 identical no-load flow assignments Net reduction: 84 lines removed (284 changed: +100 -184). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…coil-lookup branches in GetFurnaceInput The WaterToAirHeatPump loop had three nearly identical if/else-if chains for each of the three coil model variants (ParameterEstimation, EquationFit, VariableSpeedEquationFit), repeated across heating coil input, cooling coil input, heating capacity retrieval, and cooling capacity retrieval -- 12 blocks total. This replaces them with two dispatch tables (wahpHeatingCoils, wahpCoolingCoils) and two helper lambdas (readWaterToAirCoil, getWaterToAirCoilCapacity) that iterate the table to find the matching variant and call the appropriate namespace functions via function pointers. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ceInput Comment out the unused alphaFieldType parameter name to suppress -Werror=unused-parameter build failure. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…mpts Source changes: - HVACVariableRefrigerantFlow.cc: extract readFluidCtrlCommonFields to deduplicate FluidTCtrl HP/HR condenser input loops (-398 net lines) - SurfaceGeometry.cc: consolidate surface-moving loops, merge duplicate interzone warnings, and extract exposure warning helper (-81 net lines) Tooling: - Add tools/generate-dry-data.sh: extracts git history into JSON for viz - Add tools/dry-refactor-viz.html: D3.js treemap dashboard showing refactoring progress with animated playback and particle effects - Add tools/dry-refactor-done.txt: tracks completed functions so the auto-dry-refactor agent skips them - Harden auto-dry-refactor agent: remove Edit/Write tools, add strict rules against self-modification, require done-list check before selection - Update dry-refactor agent: use ninja directly, specify build-normal Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…lFlag output variable blocks in RefrigeratedCase Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…on and custom gas property loading in GetMaterialData The SpectralAndAngle validation block was copy-pasted 3 times (for Trans, FRefl, BRefl curves) with ~35 lines each. Extracted into validateSpecAngCurve lambda called 3 times. This also fixes a latent copy-paste bug where blocks 2 and 3 referenced field index 5 instead of 6 and 7 in their error messages. The custom gas property loading + validation was duplicated between WindowMaterial:Gas and WindowMaterial:Gap:EquivalentLayer sections (~35 lines each). Extracted into loadCustomGasProps and calcGasNominalR lambdas. Net reduction of ~101 lines. Removed unused ICoeff and DenomRGas variables. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…on efficiency calculation in UpdateSysSizing The EndDay block of UpdateSysSizing had 6 near-identical copies of the ASHRAE 62.1 zone ventilation efficiency calculation (SP simplified procedure and VRP multi-path/single-path). Extract the common logic into a static helper function calcZoneVentEfficiency that handles both cooling and heating modes. Three of the six occurrences were cleanly replaced; the remaining three have structural differences (split compute/save, writes to wrong arrays) that prevent direct replacement without behavior change. Reduces UpdateSysSizing NLOC from 1838 to 1670 (-168). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…min-finding loops in UpdateSysSizing The EndDay ZoneSum branches for both Coincident and NonCoincident sizing had identical pairs of loops scanning cooled and heated zones to find the minimum ZoneADEffCooling and ZoneADEffHeating values. Extract the common loop body into a static helper updateMinADEffBySys, replacing 4 loop instances with 4 one-line calls. Reduces UpdateSysSizing NLOC from 1670 to 1622 (-48). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…k data copy in UpdateSysSizing The EndSysSizingCalc block copies ~24 identical cooling peak fields from SysSizing to CalcSysSizing twice: once for SensibleCooling peak and once for TotalCooling peak. Extract the common field assignments into a static helper copyCoolPeakToCalcSysSizing, reducing each 24-line copy block to a single function call. Reduces UpdateSysSizing NLOC from 1622 to 1576 (-46). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… in UpdateSysSizing DuringDay The DuringDay block saves 9 identical cooling peak condition fields into the SysSizing record twice: once when sensible cooling capacity peaks and once when total cooling capacity peaks. Extract these assignments into a static helper saveDuringDayCoolPeak. Reduces UpdateSysSizing NLOC from 1576 to 1562 (-14). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…lidation blocks in GetCurveInputData Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…t reading blocks in GetCurveInputData Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ptionalOutputLimits helpers to deduplicate unit-type validation and output-limit reading across 20 curve types Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ce TriQuadratic 27-line coeff assignments with loop, simplify Table:Lookup if-else-if chain to indexed loop Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ate-check+AddCurve blocks in GetCurveInputData Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…to consolidate coefficient loading, input limit setup, output limit reading, and validation across 17 curve types Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Move the non-coincident cooling zone data accumulation and capacity calculation out of UpdateSysSizing into a static free function, paralleling the existing accumulateNonCoinHeatZoneData helper. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Move the cooling sizing ratio application logic (mass flow scaling, timestep capacity recalculation, and zone cooling flow adjustment) into a static free function, reducing UpdateSysSizing NLOC by 65. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Move the heating sizing ratio application logic (mass flow scaling, timestep heating capacity recalculation, and zone heating flow adjustment) into a static free function, reducing UpdateSysSizing NLOC by 50. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Move the system sizing results file (.ssz) output -- header, timestep data, and coincident/non-coincident peak summaries -- into a static free function, reducing UpdateSysSizing NLOC by 99. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Deduplicate the near-identical cooling/heating OAT limit checking blocks (~100 lines) into a single static helper that is parameterized by isCoolingMode. Reduces InitVRF NLOC from 1415 to 1319. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Deduplicate the zone exhaust/inlet node search pattern that was repeated for TU inlet (exhaust) and TU outlet (inlet) node checks. Both blocks shared identical nested loops over zones and nodes. Reduces InitVRF NLOC from 1319 to 1291. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Consolidate the 4 repeated OA-mixer/direct-inlet flow-setting blocks into a single static helper parameterized by main and OA mass flow rates. Reduces InitVRF NLOC from 1291 to 1279. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Consolidate the duplicated single-gas setup code shared between WindowMaterial:Gas and WindowMaterial:Gap:EquivalentLayer loops into a single initSingleGasMaterial lambda, reducing copy-paste in GetMaterialData. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Consolidate four identical read-and-validate blocks for top/bottom/left/right opening multipliers in WindowMaterial:Screen into calls to a new readFieldAndValidateRange01 static helper. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Move the SpectralAndAngle curve validation lambda out of GetMaterialData into a static free function. This reduces the NLOC of GetMaterialData by ~51 lines and improves stack trace visibility for debugging. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Move the custom gas property loading and nominal resistance calculation lambdas out of GetMaterialData into static free functions. This reduces GetMaterialData NLOC by ~39 lines and makes these helpers visible in stack traces. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace 18 repetitive getNumObjectsFound + for-loop blocks for simple curve types with a static constexpr data table (SimpleCurveSpec) and a single loop that iterates over it. Each entry in the table captures the IDD object name, CurveType enum, number of dimensions, number of coefficients, and whether to validate unit types. This reduces GetCurveInputData from 644 to 539 NLOC (-105). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The QuadLinear and QuintLinear curve reading blocks were manually doing what readSimpleCurveFields already handles generically: setting curveType/numDims, loading coefficients, reading input limits per dimension, reading optional output limits, and checking unit types. Adding these two curve types to the simpleCurveSpecs table eliminates ~90 lines of hand-written loop code. This further reduces GetCurveInputData from 539 to 459 NLOC (-80). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Consolidate three nearly identical speed-level monotonicity validation loops (water flow, air flow, and capacity) into a single reusable static helper function. Reduces SizeVarSpeedCoil NLOC by 26. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Consolidate two near-duplicate capacity mismatch warning blocks (both-autosized and total-only-autosized) into a single parameterized helper function. Reduces SizeVarSpeedCoil NLOC by 22. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Consolidate the duplicated HPWH cooling capacity formula (which accounts for condenser pump power based on CondPumpPowerInCOP flag) into a single reusable static helper. Reduces SizeVarSpeedCoil NLOC by 5 and CCN by 2. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…t population The interior and exterior branches of the SurfaceFilter classification in GetSurfaceData had identical if/else chains checking Window/Wall/ Floor/Roof classes. Extract a static helper that takes an isInterior flag and selects the appropriate filter enums. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Move the subsurface exterior boundary condition consistency checking loop (~107 lines) into a dedicated static helper. This also consolidates the repeated format strings for the subsurface condition description (adiabatic/interzone/other) into shared local variables, reducing duplication within the extracted function. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Move the entire interzone surface matching loop (~400 lines) into a dedicated static helper function. This includes construction layer reversal checks, area/tilt/azimuth mismatch warnings, exposure validation, and blank boundary condition handling. All variables local to this logic (NonMatch, izConstDiff, izConstDiffMsg, TotLay, TotLayFound, MultFound, MultSurfNum) are now scoped to the helper. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Move the surface list building loop (~95 lines) and SurfaceFilter population into a dedicated static helper. This consolidates the construction of AllHTSurfaceList, AllExtSolarSurfaceList, window/ non-window lists, interzone lists, Kiva foundation lists, shadow obstruction flags, and SurfaceFilter classification into one cohesive function. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Deduplicate the ConsFanVarFlow cold/hot water SolveRoot error-handling pattern. Both cooling and heating branches used identical nested SolFlag checks with convergence and limit error reporting; these are now handled by a single static helper that takes the fluid label and counter references as parameters. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Deduplicate the CycFan/VarFanVarFlow PLR SolveRoot error-handling pattern. Both cooling and heating branches used identical nested SolFlag checks with convergence and limit error reporting; these are now handled by a single static helper parameterised by mode label and counter references. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Deduplicate the VarFanConsFlow PLR iteration loop that was identically repeated for cooling and heating. The while-loop with relaxation-based convergence and the max-iterations warning are now in a single static helper. Also removes unused local variables (Error, AbsError, Relax, DelPLR) that were only consumed by the extracted loops. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Deduplicate the ConsFanVarFlow flow-locked bypass logic that was repeated for both cooling and heating coils. When plant flow is locked above the needed rate, the helper computes coil output at the needed flow then blends bypass flow to correct outlet temperature and enthalpy. Also removes unused CWFlowBypass / HWFlowBypass locals. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Move classifySurfaceFilter before its caller buildSurfaceLists to fix forward-reference error, and remove unused variable in Sim4PipeFanCoil. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add SetPredefinedTables to the done list after analysis showed no DRY opportunities (all unique string assignments, CCN=3). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Consolidate 8 repeated state-clearing blocks that reset DataIsDXCoil, DataEMSOverride, DataTotCapCurveIndex, DataConstantUsedForSizing, etc. into a single static helper function. Reduces SizeDXCoil NLOC from 1095 to 1049. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Consolidate the two nearly identical multispeed evaporative condenser sizing loops (air flow rate and pump power) into a single parameterized helper function. Both loops had the same structure differing only in the per-capacity factor and field description strings. Reduces SizeDXCoil NLOC from 1049 to 1012. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Consolidate four nearly identical two-speed validation checks (evap condenser air flow, pump power, total cooling capacity, air volume flow rate) into a single parameterized helper. Each check followed the same pattern of comparing low-speed to high-speed values with severe error + fatal on violation. Reduces SizeDXCoil NLOC from 1012 to 991. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is an experimental reduction created using a DRY refactoring AI agent.
I do not expect this to ever get merged, but I'm hoping to test regressions on the CI, and want to look at a visualization of the diffs.