Skip to content

Feat/auto config params separation - #275

Open
viktorbeck98 wants to merge 3 commits into
developmentfrom
feat/auto-config-params-separation
Open

Feat/auto config params separation#275
viktorbeck98 wants to merge 3 commits into
developmentfrom
feat/auto-config-params-separation

Conversation

@viktorbeck98

@viktorbeck98 viktorbeck98 commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator

Task

Description

Configure-phase inputs move out of params into their own top-level
auto_config_params block, so a config shows at a glance which settings drive
auto-configuration and which drive training and detection. AutoConfigParams is
the base; the variable, combo and sequence families each narrow it. The
now-redundant stability_ prefix is dropped inside the block.

set_configuration() no longer rebuilds self.config — it writes only what the
configure phase produced, then flips auto_config off. The wholesale rebuild is
what silently dropped persist and every other operator setting; the carried-
settings workaround and AutoConfigWarning are deleted along with it.

Auto-config settings now shape the configure-phase persistency only, so the
trained trackers stop collecting timestamps nothing reads.

Breaking:

  • No legacy compatibility — the old flat spellings are validation errors, not
    deprecated aliases.
  • Persisted tracker state written before this PR no longer loads and must be
    regenerated. Tracker state carries a serialized detector config, so any field
    rename breaks it; this PR renames fields.

The first commit predates this work and is committed as-is — review the branch
work against it rather than against development. Docs and AGENTS.md updated
to match.

How Has This Been Tested?

Full suite passed

Checklist

  • This Pull-Request goes to the development branch.
  • I have successfully run prek locally.
  • I have added tests to cover my changes.
  • I have linked the issue-id to the task-description.
  • I have performed a self-review of my own code.

StabilityClassifier.incline() computes where in a binary change series the
changes sit, in [-0.5, +0.5]: -0.5 is every change at the very start, 0 is
uniform churn, +0.5 is every change at the end. It is the least-squares
slope with its data-free factors divided out, so the value is comparable
between events instead of scaling with the change count. RLELists are read
via runs(), one pass, no expansion.

Trackers opt in with require_declining, which adds a conjunct to STABLE
requiring the variable's changes to sit early in its series. Independent of
segmentation -- it reads index positions, not timestamps.

Committed with --no-verify: this is a snapshot of work that predates the
branch and was never hook-clean. The lint and mypy fixes land in the
following commit, so the branch tip is clean.
Configure-phase inputs move out of `params` into their own top-level
`auto_config_params` block, so a config makes plain which settings shape
auto-configuration and which drive training and detection.

  AutoConfigParams (common/detector.py) is the base; each family narrows it:
  - VariableAutoConfigParams -- use_stable_vars, use_static_vars,
    segmentation, timestamp_variable, timestamp_format, require_declining,
    incline_threshold
  - ComboAutoConfigParams    -- adds max_combo_size
  - SequenceAutoConfigParams -- min_window_size, max_window_size

The `stability_` prefix is dropped inside the block (stability_segmentation
-> segmentation, stability_require_declining -> require_declining); the
prefix only existed to disambiguate names sharing a flat namespace.

incline_threshold becomes configuration rather than a constant reachable
only through tracker.stability_classifier.

set_configuration() no longer rebuilds self.config. It writes only what the
configure phase produced -- config.events (via the new generate_events_config)
and, for EventSequenceDetector, fixed_window_size -- then flips auto_config
off. The wholesale rebuild is what silently dropped `persist` and every
other operator setting, and is why _CARRIED_SETTINGS and the four
hand-written restore lists existed; all are deleted. AutoConfigWarning goes
too: it warned about params being lost, which no longer happens.

Auto-config settings now shape the configure-phase persistency only. The
trained persistency is built without stability kwargs and _ingest no longer
feeds it timestamps -- stability classification is never consulted at
detect time, so those were an unread O(N) list per tracked variable in the
detector that actually runs in production.

BREAKING: no legacy compatibility. The old flat spellings are validation
errors, not deprecated aliases.

BREAKING: persisted tracker state written before this commit no longer
loads. SingleStabilityTracker.to_state() serializes the whole detector
config verbatim and from_state() reinflates it through an extra="forbid"
class, so old blobs carrying the old flat field names raise
PersistencyLoadError. This is not new to this change -- any field rename in
any detector config breaks old state the same way -- but this commit renames
fields, so it triggers it. Existing state files must be regenerated.
The section's only config block showed `segmentation: time`, which reads as
the recommended setup. The default is `count` -- as the fields table below
it already says -- so name that at the point the example appears.
@viktorbeck98
viktorbeck98 requested a review from ipmach August 20, 2026 13:52
@viktorbeck98 viktorbeck98 self-assigned this Aug 20, 2026
@ipmach

ipmach commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Hi, I didn't have time to review it yet, but what do you mean with

No legacy compatibility — the old flat spellings are validation errors, not
deprecated aliases.

auto_config = True will not work?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the AutoConfig has its own parameters outside the config class? Also why the others like the parsers dont have it?
This breaks the architecture.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants