Negative Selection Algorithms for Outlier detections #690
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 76645387d1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| def test_all_nsa_variants_fit_score_predict(): | ||
| X_train, X_test, y_test, contamination = _dataset() | ||
| for model in _models(contamination): |
There was a problem hiding this comment.
Convert NSA example file into an executable example
The new examples/nsa_example.py is written as a pytest test module (test_* functions with asserts) instead of a runnable example script, and it has no if __name__ == "__main__": execution path. Running this file directly will not provide the usage/demo behavior expected from files in examples/, so users do not get an actual NSA example despite the new-model example requirement.
Useful? React with 👍 / 👎.
| X_restored = np.zeros((X_selected.shape[0], n_features)) | ||
| X_restored[:, self.feature_indices_] = X_selected | ||
| return X_restored |
There was a problem hiding this comment.
Preserve original features when caching train data
When feature subsampling is active, _restore_feature_matrix fills dropped columns with zeros before inverse scaling, so X_train_original_ is not the original training set but a distorted version with omitted features forced to feature minima. Any later partial_fit retrains on this corrupted history, which can shift scaling and detector behavior unexpectedly.
Useful? React with 👍 / 👎.
| self.X_train_original_ = self.scaler_.inverse_transform( | ||
| self._restore_feature_matrix(X, self.n_features_in_) | ||
| ).copy() if X.shape[1] == self.n_features_in_ else np.empty((0, self.n_features_in_)) |
There was a problem hiding this comment.
Keep prior training data for grid partial_fit path
In the grid variant, when feature subsampling is used (X.shape[1] != n_features_in_), X_train_original_ is set to an empty array instead of the fitted training data. A subsequent partial_fit then refits only on the new batch (vstack with empty), dropping all historical self samples despite the method contract saying it combines old and new profiles.
Useful? React with 👍 / 👎.
yzhao062
left a comment
There was a problem hiding this comment.
Thanks for this contribution, @kishordgupta. Negative selection is a legitimate and citable family (Forrest et al. 1994; Gonzalez and Dasgupta 2003 for real-valued NSA; Ji and Dasgupta 2004 for V-Detector), and broadening PyOD's immune-inspired coverage is welcome. Several things need to change before this can merge.
1. The tests do not run in CI (blocker).
The tests were added as pyod/test/nsa_example.py. PyOD's CI runs coverage run --source=pyod -m pytest with no python_files override, and pytest only collects files named test_*.py by default, so this file is never collected and the roughly 1300 new lines have no automated coverage. Please:
- Rename it to
pyod/test/test_nsa.py. - Remove the duplicate:
pyod/test/nsa_example.pyandexamples/nsa_example.pyare byte-identical. Keep a runnable demo underexamples/and the real tests underpyod/test/test_nsa.py. - The current single test asserts
roc_auc_score >= 0.60on one synthetic dataset for every variant, which is a near-trivial floor. Add per-variant coverage that would actually fail if a given variant were broken: output shape, score polarity (higher means more anomalous), and a discrimination check on data with known outliers.
2. Collapse or justify the variant classes (blocker).
The module exports about 35 classes, but 20+ are one-line __init__ aliases that set self.variant and route into one shared engine (for example RRNSA, GFRNSA, MatrixNSA, PRR2NSA, AntigenNSA, NSAII, FBNSA, CNSA). As written this is one engine behind many public names, which is a long-term maintenance liability and inflates the diff. Please either reduce to the variants that are genuinely distinct algorithms, or expose the variant as a constructor parameter on a single class instead of 35 subclasses.
3. Add citations and label honestly (blocker).
The module states the classes are "faithful engineering approximations rather than line-by-line reproductions of each cited paper," yet cites no papers. For methods with specific published origins this is a problem. Please add references (Forrest 1994; Gonzalez and Dasgupta 2003; Ji and Dasgupta 2004; plus the source for any other named variant) to docs/zreferences.bib and cite them in the docstrings. Where an implementation is an approximation rather than the published method, label it "NSA-inspired" rather than using the published method name.
4. Integrate the detector into the package (blocker).
pyod/models/nsa.py is never added to pyod/models/__init__.py, so the detectors are invisible to the public API, the model registry, and ADEngine. Please export the public class(es) from pyod/models/__init__.py, confirm BaseDetector compliance (fit sets a 1-D decision_scores_ where higher means more anomalous and then calls _process_decision_scores; decision_function is implemented; __init__ only stores hyperparameters), and add the detector to the docs algorithm list and the detector count if you intend it to ship.
5. partial_fit and the benchmark.
partial_fit is exercised only by test_online_feedback_partial_fit, which (per item 1) never runs, so its correctness is unverified; please cover it once the tests are collected. The examples/nsa_family_benchmark.py script runs on synthetic make_blobs with no real dataset or baseline; either make it a meaningful benchmark (a real dataset plus at least one PyOD baseline such as IForest) or drop it.
Happy to re-review once these are addressed. Thanks again for the work.
|
Thanks for feedback, will work on it |
All Submissions Basics:
All Submissions Cores:
New Model Submissions: