feat(extremes): count-stable clustering and transferable replace#104
Draft
FBumann wants to merge 1 commit into
Draft
feat(extremes): count-stable clustering and transferable replace#104FBumann wants to merge 1 commit into
FBumann wants to merge 1 commit into
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…replace Consume the tsam fix in FZJ-IEK3-VSA/tsam#414 (closes upstream #410) so extreme periods compose with the wrapper's fan-out and transfer paths. - Always force ExtremeConfig(preserve_n_clusters=True) for the additive methods (append/new_cluster) so every independently-aggregated slice yields exactly n_clusters and results stack rectangularly. Extremes now count against the cluster budget instead of adding a data-dependent number of extra clusters. - Lift the cluster_on + method="replace" rejection when the installed tsam can transfer the hybrid representative (stores extreme_replacements). - Both behaviours are feature-detected: no-ops on released tsam (which lacks the flag), so existing behaviour and CI are unchanged. They activate automatically once tsam_xarray runs on a tsam that ships the fix — which requires the v4 pipeline architecture the fix is built on. Adds test/test_extremes_composability.py: a 30-case fuzz over additive extremes on sliced data (asserts the fallback count contract today, tightens to exact-count once the flag is present) plus flag-gated count-stability and replace-transfer tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
FBumann
force-pushed
the
feat/extremes-count-stable
branch
from
July 23, 2026 11:26
dcdfb23 to
cdae729
Compare
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.
Makes extreme periods composable with the wrapper's fan-out (many independent slices → one rectangular array) and transfer (
cluster_on,apply()) paths, by consuming the upstream tsam fix FZJ-IEK3-VSA/tsam#414 (closes upstream #410).Rebased onto the v4-compatible
main(#105), so the wrapper now runs on both released tsam 3.4.x and tsam v4.What changed
append/new_cluster), the wrapper now always forcesExtremeConfig(preserve_n_clusters=True). tsam otherwise appends a data-dependent number of extreme clusters (criteria that resolve to the same period, or an extreme that coincides with a medoid, get deduplicated), so independently-aggregated slices ended up with different cluster counts and failed to stack — the_validate_consistent_cluster_countserror. With the flag, every slice yields exactlyn_clusters. Extremes count against the cluster budget rather than adding to it.replace. Thecluster_on+method="replace"rejection is lifted when the installed tsam stores the injection (extreme_replacements) and can replay it throughapply(). Previouslyreplacesilently degraded to the plain medoid on transfer, so the wrapper hard-rejected it.Feature-detected — safe on every supported tsam
Both behaviours are gated on introspecting the installed tsam (
preserve_n_clustersonExtremeConfig,extreme_replacementsonClusteringResult):develop-v4) neither exists → the new code is a no-op. Existing behaviour, thereplacerejection, and CI are unchanged.Validation
replace-transfer tests now execute and pass end-to-end. Directly confirmedpreserve_n_clusters=Truepins the count across datasets andreplaceround-trips exactly throughapply()and JSON.Remaining dependency
The only thing left is tsam releasing #414 (currently an open PR on
develop-v4). Until a tsam version ships withpreserve_n_clusters/extreme_replacements, these paths stay dormant (no-op) on the tsam that CI installs. Kept as draft for that reason — ready to mark for review once #414 lands, or now if you'd prefer to merge the staged wrapper-side changes ahead of it.Tests
test/test_extremes_composability.py:== n_clustersassertion where the flag is present.preserve_n_clusters=Falseis overridden) andreplace-transfer tests (cluster_on+apply()round-trip).test_replace_extremes_rejectedis nowskipifthe running tsam transfersreplace.Refs FZJ-IEK3-VSA/tsam#414, FZJ-IEK3-VSA/tsam#410.
🤖 Generated with Claude Code