Repository navigation
Make stopping on the claim's success verdict unconditional - #53
Merged
Merged
Conversation
#49 added `stop_on_success` as a Controller flag. It should not be a flag: the security claim decides success, so running on after a win is never what an ASR measurement wants. Nothing outside the framework ever set it, in either superred-modules or superred-experiments. Removing it removes the problem #52 existed to fix. `identity_hash()` hashes a dict, so keying the flag changed the digest even at its default and orphaned every results tree ever written; the fix for that was to key it only when disabled, which is machinery that only earns its keep if the disabled state exists. With one regime there is no second directory to distinguish, no identity key, no ExperimentMeta field and no experiment_block entry. It also deletes states rather than adding them. Three tests described scenarios that can no longer occur and are gone: a task that wins then times out, a task that wins then fails, and the opt-out itself. The first of those was a real source of confusion in a certified results tree, where 8 tasks the attacker had already won were recorded as `stop_reason="timeout"`. A TaskResult with success=True now always carries stop_reason="success". Five other tests used the flag only to keep a multi-run scenario alive; they now use a task that does not win, which is what they were really about. What is lost is measuring attack RELIABILITY, k of N attempts succeeding rather than whether any did. Nothing in the codebase does that, and superred measures ASR. If it is wanted later it needs its own design, not a flag on this loop. Net 137 deletions against 64 insertions. 591 tests pass.
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
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 #52, which is closed. Follow-up to #49, which is merged.
Why the flag should not exist
#49 added
stop_on_successas a Controller argument. It should not be a flag. The security claim decides success, so running on after a win is never what an ASR measurement wants, and nothing outside the framework ever set it — zero references insuperred-modulesandsuperred-experiments, and only 8 test call-sites.Removing it removes the problem #52 existed to fix.
identity_hash()hashes a dict, so keying the flag changed the digest even at its default (ef26fece→5b18e4e4) and would have orphaned every results tree ever written. The fix was to key it only when disabled, which is machinery that only earns its keep if the disabled state exists. With one regime there is no second directory to distinguish: no identity key, noExperimentMetafield, noexperiment_blockentry, no frozen-hash guard needed.It deletes states rather than adding them
Three tests described scenarios that can no longer occur, and are gone:
The first was a real source of confusion in a certified results tree: 8 tasks the attacker had already won were recorded as
stop_reason="timeout", indistinguishable from tasks that ran out of clock. That state cannot arise now. ATaskResultwithsuccess=Truealways carriesstop_reason="success".Five other tests used the flag only to keep a multi-run scenario alive. They now use a task that does not win, which is what they were actually about.
What is lost
Measuring attack reliability — k of N attempts succeeding, rather than whether any did. Nothing in the codebase does that, and superred measures ASR. If it is wanted later it deserves its own design rather than a flag on this loop.
Net effect
64 insertions, 137 deletions. The controller loses a parameter, an attribute and a branch; the docs lose a knob to explain.
Checks
591 passed·ruff check/formatclean ·mypyclean on 25 source files.