perf: derive the objective scale from the model instead of fixing it - #123
Merged
Conversation
#104 introduced OBJECTIVE_SCALE = 1e6 and review asked for the proof that a constant is the right choice for every case. There is none, and the constant is demonstrably not doing what its comment claims. The raw coefficients are set by the request, prices per Wh, a demand rate per W, the penalty base they scale from. Across the stored cases the largest one moves by three orders, 1.6e-1 where market prices go negative against 3e2 on the peak levelling cases, so one factor cannot place them all. With 1e6 applied, the model handed to the solver tops out anywhere between 1.6e5 and 3e8, and six of nineteen cases go past 1e6. So the factor is derived per model: the largest coefficient is placed at OBJECTIVE_TARGET. OBJECTIVE_SCALE stays as an override, now defaulting to None, which is what the scaling test uses to compare two scalings of the same model. The new test asserts the placement over every stored case, which is the assertion a fixed factor cannot carry. Anchor on the largest coefficient, never on the smallest. Tried the other way first, aiming the smallest at a floor: the smallest coefficient is a strategy tie breaker and deliberately tiny, so anchoring there drags the whole objective down until it sits orders below a constraint matrix carrying a big M of 1e6. Measured over 335 requests from the slow request dump, that costs 3 of them their solution outright and runs others to 30 s against a 10 s limit. Measured, in the form it ships: it is a wash. 335 requests at the production limit of 10 s, all Optimal on both sides, median 7.65 s to 8.26 s, total 1990 s to 2018 s, one request worse by 0.0002 and one better by 0.0034 in currency among the 171 where both runs converged. No golden case moves. That is the honest result. This does not make anything faster, it replaces a constant nobody could justify with one line that follows from the model, and buys an invariant that can be asserted. Co-Authored-By: Claude Opus 5 (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
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.
Follows up the third review comment on #115, which was about #104 rather than about that PR:
There is no proof, and the measurement says the constant is not doing what its comment claims.
What a fixed factor actually does
The raw objective coefficients are set by the request: prices per Wh, a demand rate per W, the penalty base they scale from. Across the stored cases the largest raw coefficient moves by three orders. With
1e6applied, this is the model the solver receives:Six of nineteen land above 1e6, the peak levelling cases by two and a half orders. One constant cannot place a quantity that itself moves by three orders, which is the whole of the objection.
What this does instead
objective_scale()puts the largest coefficient atOBJECTIVE_TARGET, one line derived from the model.OBJECTIVE_SCALEstays as an override and now defaults toNone; the existing scaling test uses it to pin a factor and compare two scalings of the same model. The new test asserts the placement across every stored case and every strategy, which is the invariant a fixed factor cannot carry.The within-model span is a separate matter and not fixable by any factor: it runs to 1e8 on
018-high-soc-initial, wheremin_import_priceis -9.9e-7 and the tie breaker coefficients collapse toward zero while the penalties sit at 1.4e6. That is a modelling question.Anchor on the largest, never on the smallest
Worth recording since it looks like the obvious alternative. Aiming the smallest coefficient at a floor, so the small terms clear CBC's tolerance, backfires: the smallest coefficient is a strategy tie breaker and deliberately tiny, so anchoring there drags the entire objective down until it sits orders below a constraint matrix that carries a big M of 1e6. Measured over 335 requests from the slow request dump:
Three requests lose their solution outright and others run three times past a 10 s limit, because a model that ill conditioned keeps CBC in presolve and the root LP, where the time limit is not tested.
What it is worth
A wash. Over the same 335 requests at the production limit of 10 s, every one Optimal on both sides: median 7.65 s to 8.26 s, total 1990 s to 2018 s. On money, among the 171 where both runs converged, one request is worse by 0.0002 and one better by 0.0034 in currency. No golden case moves.
So this makes nothing faster. It replaces a constant nobody could justify with one line that follows from the model, and buys an invariant that can be asserted. If that is not worth the churn, closing it is a perfectly good outcome and the measurement above is the answer to the review comment either way.