represent ECVC and SCVS state with persistency (#255) - #274
Open
viktorbeck98 wants to merge 1 commit into
Open
Conversation
Both detectors kept their learned count vectors in a plain in-memory set, so a trained model was lost on restart. They now store them as keys in EventPersistency.events_seen, the pattern EventSequenceDetector already uses, which gives them save, load and auto-load for free. ECVC derives its matrix and threshold from those vectors in _derive(), called after training and after a load. The vectors are sorted first: restored keys are strings whose set iteration order is hash-randomized per process, and the seeded shuffle splits train from validation by that order, so sorting is what makes a restored model equal a trained one. Keys carry the window size, since a count vector's length is max(EventID) + 1 and says nothing about the window it was counted over. Restoring state at a different window_size now logs a warning instead of silently alerting on every window. The count vector and sequence codec helpers move to utils/sequence_encoding.py so no detector imports from another. As a result build_count_vec is no longer importable from scvs_detector. Co-Authored-By: Claude Opus 5 <[email protected]>
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.
Task
Closes #255.
Description
post_train's body moved to_derive(), calledafter training and after a load. It sorts the vectors first: restored keys are
strings whose set iteration order is hash-randomized per process, and the seeded
shuffle splits train from validation by that order. The sort is what makes a
restored model equal a freshly trained one.
max(EventID) + 1and says nothing about the window it was counted over, so restoring state at a
different
window_sizeused to mean every vector missed and every windowalerted. It now logs a warning.
post_trainused to freetrain_seqs;it can't now — those vectors are the persisted model. Memory grows with the
number of distinct training windows.
utils/sequence_encoding.pyso no detector importsfrom another. Consequence:
build_count_vecis no longer importable fromscvs_detector.ECVCOp.build_count_vecis unchanged.How Has This Been Tested?
pytest --run-ignored— 700 passed, including the ECVC and SCVSaudit.logend-to-end regressions, which detect the same log IDs as before. New coverage in
test_persist_integration.py: save/reload per detector, codec round-trip, thewindow-size warning, and an assertion that a reloaded ECVC matrix and threshold
match a trained one
Checklist
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.