Feat/save persistency#144
Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds full PersistencySaver class to persistency_saver.py with save(), load(), start(), stop() methods using fsspec for backend-agnostic I/O. Includes 8 new TDD tests covering save/load round-trip via in-memory fs. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ve error messages, make stop() idempotent
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…boDetector, NewEventDetector
…tent - PersistencySaver.save() now calls _safe_event_data_kwargs() to exclude non-JSON-serializable values (e.g. callables like converter_function) before writing metadata.json, preventing silent save failures. - PersistencySaver.stop() is now idempotent: returns early if _timer is None and unregisters the atexit hook after the first call, eliminating spurious I/O-on-closed-file tracebacks during test teardown. - EventTracker.load() uses strict_map_key=False with object_pairs_hook to restore tuple dict keys that msgpack deserializes as lists. - SingleStabilityTracker.from_state() converts list elements in unique_set back to tuples to preserve hashability after msgpack round-trip. - Adds test_save_and_reload for NewValueComboDetector to verify full save/reload cycle with callable event_data_kwargs.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ns examples Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add ingest callback hook to EventPersistency and wire _check_event_count into PersistencySaver so state is saved automatically after N events, independently of the background timer. Includes three tests covering at-threshold, before-threshold, and re-trigger-after-reset behaviour.
|
@ipmach ready for review |
Resolve conflict in core.py: keep both _Stoppable protocol (from this branch) and TrainBuffer class (from development), plus use_config_data_as_training and buffer_train additions from development.
bd149da to
57830a2
Compare
Use pandas' to_parquet/read_parquet instead of importing pyarrow directly. pyarrow remains the underlying engine (declared dependency), but no source file imports it anymore. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
Why there is a protocol, and what is the purpose of this?
There was a problem hiding this comment.
It’s only there to keep the dependency clean. core.py only needs to call .stop() on whatever saver-like object gets attached. So we don't need to make core dependent on smth from the detector persistency
There was a problem hiding this comment.
This file is saturated. It needs some refactor.
All the methods outside the CoreDetector that are not call in the core detector should not be in this file as they are not part of it. I suggest to move this to a persist file.
_register_persistency method should be outside of the class. It takes to much space in the class an does not need to be part of it as it never use the self pointer.
Suggestion:
def init_persistency(config: CoreDetectorConfig, persistency: EventPersistency) -> EventPersistency:
config = cast(CoreDetectorConfig, self.config)
if config.persist is None:
return
p = config.persist
saver = PersistencySaver(
persistency,
PersistencySaverConfig(
path=f"{p.path}/{self.name}",
save_interval_seconds=p.interval_seconds,
events_until_save=p.events_until_save,
auto_load=p.auto_load,
storage_options=p.storage_options,
),
)
saver.start()
return saver
class CoreDetector:
........
def _register_persistency(self, persistency: EventPersistency) -> None:
self.saver = init_persistency(self.config, persistency=persistency)
There was a problem hiding this comment.
I suggest to treat every persistency method as its own library. (Similar as the schemas)
So, everytime you need a method you do:
import detectmatelibrary.utils.persistency as persistency
persistency.get_configure_variables(.....)
If you follow this syntax then is clear all the methods of each file that is part of the persistency and we have a better separation.
For example in the NewValueDetector:
class NewValueDetector(CoreDetector):
"""Detect new values in log data as anomalies based on learned values."""
def __init__(
self,
name: str = "NewValueDetector",
config: NewValueDetectorConfig = NewValueDetectorConfig()
) -> None:
if isinstance(config, dict):
config = NewValueDetectorConfig.from_dict(config, name)
super().__init__(name=name, buffer_mode=BufferMode.NO_BUF, config=config)
self.config: NewValueDetectorConfig # type narrowing for IDE
self.persistency = persistency.EventPersistency(
event_data_class=persistency.EventStabilityTracker,
)
# auto config checks if individual variables are stable to select combos from
self.auto_conf_persistency = persistency.EventPersistency(
event_data_class=persistency.EventStabilityTracker
)
self._register_persistency(self.persistency)
So I do not need to look in the imports where each class from persistency comes from. Same as
import numpy as np
np.load() # I know this load comes from numpy
but if I do:
from numpy import load
load() # I need to go to the top of the document to know where this comes from
There was a problem hiding this comment.
files that are only use in persistency, should be inside persistency module even if they are generic. Example: RLE_list.py should be inside persistency.
So the dependency lines can be easily draw as:
(DetectMate) <------ (Persistency)
DetectMate dependes on the persistency module, but not in viceversa. It will also help if persistency has a clear API to better split between the two so the module is more modular.
(DetectMate) <---- (Persistency API ) <--- (Persistency)
Same thing as the schemas, where the division is clear:
(DetectMate) <--- (schemas/_classes.py) <--- (schemas methods)
This will make the code more clear and would helps us to later refactor the Detector class or methods to reduce the replicated code
There was a problem hiding this comment.
Lets leave it like this for now, but I think this documentation can be a little hard to follow if you dont know how the persistency works
ipmach
left a comment
There was a problem hiding this comment.
The persistency itself looks good, but it should be a more clear separation between the persistency module and the rest of DetectMate.
Otherwise we may run into future problems to maintain the code.
Lifts the persistency setup out of CoreDetector into a module-level init_persistency helper, expands the persistency package's public API so callers use the `persistency.X` namespace pattern, and rewrites the persistency doc with a concepts-first structure. Also moves RLE_list inside the persistency package since it has no consumers elsewhere, and documents why _Stoppable is a Protocol (dependency direction). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
@ipmach thanks for your comments, much cleaner now. Everything has been addressed :) |
Task
Closes #140
Description
Adds a full state persistency system to DetectMateLibrary so detectors can checkpoint and resume their internal state across runs.
Core protocol
dump/loadprotocol onEventDataStructure, implemented forEventTracker(msgpack),EventDataFrameandChunkedEventDataFrame(Parquet + msgpack header), andSingleStabilityTracker(to_state/from_state).PersistencyLoadErrorfor load failures.PersistencySaver (
src/detectmatelibrary/utils/persistency/persistency_saver.py)s3/gcs/azure)._SaveTimerandevents_until_saveevent-count threshold.stop()that joins the timer thread;atexitcleanup registered/unregistered correctly.event_data_kwargs(e.g. callables likeconverter_function) are filtered before writingmetadata.json.Config + detector wiring
PersistConfigmodel andpersistblock onCoreDetectorConfig, integrated into config serialization and theMissingParamswarning, and preserved acrossset_configuration()rebuilds.CoreDetector._register_persistency()helper wired intoNewValueDetector,NewValueComboDetector, andNewEventDetector.EventPersistencygains an ingest callback andevents_since_savecounter to drive event-count saves.Componentgains a context-manager protocol and_Stoppableprotocol for clean saver shutdown.Other
fsspec,msgpack,pyarrow(with optionals3/gcs/azureextras).docs/auxiliar/persistency.md,docs/detectors.md, andAGENTS.md(renamed fromCLAUDE.md).from_polarshelpers and theTrainBufferclass from the development merge.How Has This Been Tested?
tests/test_utils/test_persistency_dump_load.pyPersistencySavertrigger/save/load tests (timer,events_until_save, idempotent stop):tests/test_utils/test_persistency_saver.pyPersistConfigconfig-roundtrip tests:tests/test_common/test_persist_config.pytests/test_detectors/test_persist_integration.pyfrom_polarshelper tests:tests/test_helper/test_from_to.pypytest.Checklist