v3.6.6: estimator, numerical and CLI fixes - #749
Merged
Merged
Conversation
FIX: Prevent false-positive Claude Code detection in pyod info (#715)
* Respect verbose in DeepSVDD training output
DeepSVDD documents `verbose` ("Verbosity mode", default 1) and stores it
on the estimator, but the training loop prints the per-epoch loss
unconditionally. `self.verbose` is never read, so `DeepSVDD(verbose=0)`
still writes one line per epoch - 100 lines at the default `epochs`, for
every fit in a benchmark loop.
With epochs=3:
verbose=0 -> 3 epoch lines printed
verbose=1 -> 3 epoch lines printed
The print is now gated on `self.verbose == 1`, which is how AnoGAN and
LUNAR already gate their per-epoch output. The default is 1, so nothing
changes unless the caller asked for silence.
* Print epoch logs for any non-zero verbose
Gating on `verbose == 1` made `DeepSVDD(verbose=2)` quieter than
`verbose=1`, which inverts the usual meaning of the setting. Suppress the
per-epoch line only when verbosity is 0, and cover mode 2 in the test.
`GMM.fit` is `fit(self, X, y=None)`, but its docstring documents a third
argument:
sample_weight : array-like, shape (n_samples,)
Per-sample weights. Rescale C per sample. Higher weights
force the classifier to put more emphasis on these points.
Passing it raises `TypeError: GMM.fit() got an unexpected keyword
argument 'sample_weight'`.
The text is copied from `OCSVM.fit`, which does take the argument - "C"
is the SVM penalty parameter and has no counterpart in a Gaussian
mixture. It also cannot simply be forwarded: `sklearn.mixture
.GaussianMixture.fit` is `fit(self, X, y)` and has no `sample_weight`.
Only the stale documentation is removed; OCSVM is untouched.
Signed-off-by: Zhu yizhang <95731595+godarrenw@users.noreply.github.com>
Signed-off-by: Zhu yizhang <95731595+godarrenw@users.noreply.github.com>
`BaseDetector.fit` documents its contract as
Returns
-------
self : object
but the deep learning base ends `fit` on `_process_decision_scores()`
without returning, so every detector built on it returns None:
VAE().fit(X) -> None
AutoEncoder().fit(X) -> None
DeepSVDD().fit(X) -> DeepSVDD (defines its own fit)
IForest().fit(X) -> IForest
That breaks the chained form these estimators otherwise support -
`clf.fit(X).predict(X)` raises AttributeError on None - and any sklearn
utility that relies on `fit` returning the estimator.
Adds the missing return and the Returns section the base class already
documents.
`VAE.__init__` stored the validator's return value rather than the
argument:
self.logvar_clip = self._validate_logvar_clip(logvar_clip)
and `_validate_logvar_clip` ends with `return float(lower), float(upper)`,
so the attribute is a freshly built tuple. sklearn's `clone()` checks
that the constructor left its parameters untouched using an identity
comparison, so this fails outright:
RuntimeError: Cannot clone object VAE(...), as the constructor either
does not set or modifies parameter logvar_clip
which means `VAE` cannot be used with anything that clones - GridSearchCV,
cross_val_score, or a Pipeline being refit.
The argument is now stored as given. Validation still happens in
`__init__`, so an invalid `logvar_clip` is still rejected at construction
time and the existing config tests are unaffected. The normalised floats
move to `self.logvar_clip_`, derived in `build_model`, which is what the
model and the training loss now read.
`Sampling.fit` resolves a fractional `subset_size` by writing the
absolute count back onto the estimator:
self.subset_size = int(self.subset_size * n_samples)
so the constructor argument is destroyed on the first fit. Two
consequences:
* refitting on a differently sized dataset silently reuses the stale
count. With `subset_size=0.2`, fitting on 100 rows samples 20, and a
refit on 400 rows samples 20 again instead of 80;
* `get_params()["subset_size"]` returns 20 rather than the 0.2 that was
passed, so a `clone()` taken after a fit is not the estimator the user
configured. sklearn requires `__init__` parameters to be left as they
were given.
The resolved value is now a local, and `self.subset_size` is left alone.
While rewriting the surrounding lines, the `ValueError` for an
out-of-range fraction was missing its `%` operand, so it reported the
literal "subset_size=%r must be between 0.0 and 1.0". It now interpolates
the value, like the integer branch just above it already does.
`PyODKernelPCA.__init__` takes `n_components` but does not pass it to
`super().__init__()`, so the inner `KernelPCA` keeps its default of
`None`. It is the only one of the sixteen parameters that is not
forwarded.
`KPCA` exposes the argument publicly and documents it, so
`KPCA(n_components=3)` silently computes every component instead of 3:
KPCA(n_components=3).fit(X) # X is (420, 8)
inner n_components -> None
components computed -> 419
transform(X).shape -> (420, 419)
The scores are unaffected, because `decision_function` slices
`x_transformed[:, :n_selected_components_]` afterwards and the surplus
components contribute nothing. What it costs is time and memory: at
n=2000 with `n_components=3` the fit goes from 8.60s to 0.50s and the
transform matrix from 2000x1999 (32.0 MB) to 2000x3, because with the
inner value left at `None` sklearn always takes the dense `eigh` path.
`remove_zero_eig=False` is also ineffective today, since sklearn forces
zero-eigenvalue removal when `n_components is None`.
Checked `decision_scores_` before and after across seven configurations
(`n_components` 1/3/100, `n_selected_components`, `eigen_solver="arpack"`,
`remove_zero_eig=True`, `kernel="linear"`, and defaults): the largest
difference is 6.9e-14.
LOCI stored the constructor argument k directly into self.threshold_ and never called _process_decision_scores(), so contamination was accepted but completely ignored when deriving labels_. Store k on the instance, use it in the score loop where it belongs, and let the base class compute threshold_ from contamination like every other detector. Fixes #194
`pearsonr_mat` builds an `(n_row, n_row)` matrix and fills it pairwise,
but the unweighted branch runs its outer loop over `n_col`:
if w is not None:
for cx in range(n_row):
...
else:
for cx in range(n_col):
The weighted branch directly above uses `n_row`, and the docstring
describes the result as "Row-wise pearson score matrix" of shape
(n_samples, n_samples).
When there are fewer columns than rows, every pair with `cx >= n_col` is
skipped and those cells keep the value the matrix was initialised with,
so the function reports a correlation of exactly 1.0 for pairs it never
looked at. On a (50, 4) matrix, 2070 of 2500 entries are wrong, with
errors up to 2.0.
Output is unchanged whenever `n_col >= n_row - 1`, and the weighted
branch is untouched. `n_col` has no remaining reader, so it goes too.
The existing test uses a (10, 20) matrix, which is in the unaffected
range, and only checks the shape; the added test compares every entry
against `scipy.stats.pearsonr` on an (8, 3) matrix.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Coverage Report for CI Build 35170878889Coverage increased (+0.05%) to 92.625%Details
Uncovered Changes
Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
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
8 participants
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.
PyOD 3.6.6
A maintenance release focused on estimator compatibility, numerical correctness, and documentation. No new detector or dependency is introduced.
Bug fixes
kfor cloning and usecontaminationto derive the fitted threshold and binary labels.kstill controls the radius-scan early stop. #707, fixes #194.pearsonr_matutility when there are more rows than features; previously some correlations incorrectly remained 1. #731.BaseDeepLearningDetector.fit. #740.logvar_clipas supplied to the constructor sosklearn.cloneworks, while validating the training value separately. #738.subset_sizeacross fits instead of replacing the constructor argument with the first computed sample count. #737.n_componentsto the underlying estimator. #730.verbose=0. #735.pyod install skillcreated its skill directory. Detection checks the executable or user configuration instead. #723, fixes #715.Documentation and tests
sample_weightentry fromGMM.fitdocumentation. #741.score_to_labelas returning binary labels, not probabilities. #733.CITATION.cffonmaster, alongside contributor-workflow ignore rules and thetodo/drop box.Behavior notes
LOCI's binary labels now follow the documented contamination-based threshold rather than a fixed
kcutoff. Raw LOCI scores for a fixedkare unchanged by this fix; tied scores can still make the labeled fraction differ from the requested contamination.Kernel PCA scores can change when
n_componentsis specified because the limit is now honored. Sampling refits with a fractionalsubset_sizenow use the current training-set size. Previously uncomputed Pearson row pairs now receive their actual correlation values.The ROD duplicate-weighting proposal #746 is not included.
Contributors
Thanks to @Mohit-Ak, @VenishPaneliya, @Iams4kura, @busysofa15, @godarrenw, and @dishasharma23-prog.
Full changelog: v3.6.5...v3.6.6