Repository navigation
Keep tf_adjustment_weight 0.0 through the settings and creator round trip - #3345
Open
赵瑞哲 (zhaoruizhe81) wants to merge 1 commit into
Open
赵瑞哲 (zhaoruizhe81) wants to merge 1 commit into
赵瑞哲 (zhaoruizhe81) wants to merge 1 commit into
Conversation
…trip ComparisonLevel.as_dict() skipped tf_adjustment_weight when it was exactly 0, but the constructor default is 1.0, so any level configured with the documented "no adjustment" weight 0.0 silently came back as 1.0 after a settings dict / model JSON / comparison creator round trip, and save_model_to_json() wrote 1.0. Always write the weight when the level has TF adjustments (a weight of 1.0 was already written, so existing saved models are unaffected) and add regression tests for the as_dict round trip, the comparison creator path, and end-to-end predict. Fixes moj-analytical-services#3344
This branch has not been deployed
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.
Type of PR
Is your Pull Request linked to an existing Issue or Pull Request?
Fixes #3344
Give a brief description for the solution you have provided
What
ComparisonLevel.as_dict()now always writestf_adjustment_weightwhen the level has TF adjustments, instead of skipping exactly-zero weights. Adds three regression tests: anas_dict()round trip, the comparison creator path, and an end-to-endpredict()check.Why
The constructor default for
tf_adjustment_weightis 1.0, so a level configured with the documented "no adjustment" weight of 0.0 silently came back as 1.0 after any settings dict / model JSON / comparison creator round trip, andsave_model_to_json()wrote 1.0 with no warning. This contradicts the docs ("A weight of zero is equivalent to turning TF adjustments off") and made the zero-weight branch in_tf_adjustment_sql()unreachable. It has been a problem on first load since 4.0.7, whencreate_comparison_dict()started callingas_dict().Only levels with weight exactly 0.0 change: they now get an explicit
"tf_adjustment_weight": 0.0in the output dict. A weight of 1.0 was already written before this change, so existing saved models are unaffected.How I verified
68ced2a4(the base of this branch): before the change it exits 3 (RESULT: BUG PRESENT— weight saved as 1.0,mw_tf_adj_first_namenon-zero); with the change it exits 0 (RESULT: OK— weight 0.0, adjustment 0.0).tests/test_tf_adjustment_weight_zero.pyall fail on unmodified master and pass with this change (checked by stashing and re-applying the one-line fix).conftest.py).ruff checkandruff format --checkpass on both changed files.AI assistance
Per the contributing guidance: this PR was prepared with AI assistance — the patch, the tests and the local verification runs above were produced with the help of an AI coding agent, and I reviewed and submitted the result.
PR Checklist