Use IS.convert_cost_coefficient for unit-system normalization#91
Open
luke-kiernan wants to merge 3 commits intomainfrom
Open
Use IS.convert_cost_coefficient for unit-system normalization#91luke-kiernan wants to merge 3 commits intomainfrom
luke-kiernan wants to merge 3 commits intomainfrom
Conversation
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The 4 per-system-unit helpers in component_utils.jl each dispatched
on Val{IS.UnitSystem.X} through 3 method bodies. With the new
AbstractUnitSystem types and IS.convert_cost_coefficient (cost
coefficient = inverse of power-value ratio, exponent for quadratic),
each helper collapses to a single call. Net -137 lines.
Signatures move from IS.UnitSystem to IS.AbstractUnitSystem; tests
updated to pass IS.NaturalUnit() etc. instead of enum values.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Uses test mocks to run JET.@report_opt + @Btime + @code_warntype on the cost-objective construction path. Empirically backs the rung-2 claim in issue #90. JET and BenchmarkTools intentionally not in test/Project.toml; install them locally per the script header before running. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
|
Performance Results This branch |
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
1 participant
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.
Replaces the
Val{IS.UnitSystem.X}()cost-coefficient dispatch with the newIS.convert_cost_coefficientprimitive, eliminating the value-to-type boundary inside the cost-objective hot path. Net -137 lines insrc/utils/component_utils.jl; 12 method bodies collapse to 4 one-liners.Stacks on:
lk/units-domain-agnostic-is4— addsIS.AbstractUnitSystem, parameterizesCostCurve/FuelCurveonU, addsIS.convert_cost_coefficient.lk/units-fold-psu— sweeps PSY for the new types.Pinned via
[sources]inProject.tomlandtest/Project.toml.Summary
bee133c— pin IS tolk/units-domain-agnostic-is4.d1e831b— rewrite the 4 cost-coefficient helpers incomponent_utils.jlto delegate toIS.convert_cost_coefficient. Public function names and call-site behavior preserved; only internals change. Signatures move fromIS.UnitSystemtoIS.AbstractUnitSystem. Test files swept forIS.UnitSystem.X → IS.X().b08af0e— addscripts/units_dispatch_profile.jlfor empirically validating type-stability claims (see Type stability of cost-coefficient conversion: where it is, where it isn't #90). JET + BenchmarkTools are local-only deps; install per the script header.Type stability
Issue #90 documents the full picture. Short version:
Val{}-on-runtime-enum (the old antipattern) is gone.Uthrough the conversion math automatically.@code_warntypeconfirmspower_units::Core.Const(NU)inside the specialization — conversion folds at compile time.add_variable_cost_to_objective!call. Cheap to address later viaUnionfield typing if profiling flags it; not blocking.Test plan
convert_cost_coefficient(identity, all 6 cross-system pairs, exponent for quadratic, round-trip identity, negative-exponent piecewise case).scripts/units_dispatch_profile.jl) confirms: 1 dynamic dispatch site (entry point, expected), full type stability past entry, ~968μs / 31k allocs for N=50 components × 24 timesteps.🤖 Generated with Claude Code