Skip to content

Fix/open issues - #47

Merged
jpeeples67 merged 20 commits into
mainfrom
fix/open-issues
Sep 20, 2026
Merged

jpeeples67 merged 20 commits into
mainfrom
fix/open-issues

Conversation

@jpeeples67

Copy link
Copy Markdown
Contributor

No description provided.

jpeeples67 and others added 20 commits September 19, 2026 19:56
- Add .gitattributes (text=auto eol=lf) so Windows/WSL checkouts stop
  showing every file as modified.
- Use HTTPS URLs for the SAR-CNN and SARATR-X submodules so a plain
  clone/CI can fetch them without SSH keys.
- Only publish to PyPI on tag pushes (guard was commented out, so every
  push attempted a release).
- Stop tracking the results/Padim/MSTAR/latest symlink and ignore
  results/, docs/_build/, and build artifacts.

Refs #14

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- environment.yaml pinned python 3.10 / anomalib 1.2.0 while the code
  uses anomalib 2.x APIs and pyproject requires Python >= 3.13. Rewrite
  it to mirror pyproject.toml (installs the package editable).
- Bump anomalib>=2.6.2 and ultralytics>=8.4 and regenerate uv.lock. All
  anomalib imports SARIAD relies on were checked against 2.6.2.
- Trim dependencies: drop unused/mis-declared ones (dotenv, pynvim, pip,
  kornia/lightly/einops/... already provided by anomalib), move Sphinx
  packages to a `docs` extra, add `dev` (pytest) and `extras` (clip,
  openvino, onnx) extras, declare direct deps that were only implicit
  (pyyaml, requests, scipy, tqdm, torchmetrics).
- Guard archive extraction against path traversal (zip/rar/tar), using
  tarfile's "data" filter, matching the upstream anomalib hardening.

Fixes #16

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- Importing SARIAD.pre_processing crashed on CPU-only machines because
  the SAR-CNN weights were torch.load()ed at import time without
  map_location, and any missing submodule raised ImportError (only
  FileNotFoundError was handled), breaking NLM/MedianFilter/Default too.
  Load the network lazily, on the CPU, on first use, with a clear error
  when the SARCNN_SRC submodule is not checked out.
- The network output was never mapped back from the log domain
  (postprocessing_net2int was unused), and the log was taken of the
  already-normalized image. Denoise the raw intensity, map back, then
  apply the model's resize/normalization. Clamp before the log so zero
  pixels don't give -inf.
- Drop the unused noise_seed/random_stream/denoised_output_batch and the
  stray print in Default.

Fixes #6

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- Datamodules (MSTAR, HRSID, SSDD) now accept `path`, `num_workers` and
  any other anomalib `Folder` argument, so the parameters in the YAML
  `datasets:` block actually reach the constructor (default.yaml passed
  `path`/`num_workers`, which raised TypeError). The dataset root can
  also be set with SARIAD_DATASETS_PATH. Drop the redundant setup() calls
  in the constructors and runner (setup() re-splits when not called by
  the Trainer; the Engine calls it itself).
- run.py: read the top-level YAML keys (it looked for a non-existent
  `global:` block), honor `seed` (seed + run index) and `benchmark_runs`,
  write each repeated run to its own folder instead of overwriting, log
  failures with tracebacks, and don't crash when every run of an
  experiment fails. Resolve classes through SARIAD.datasets /
  SARIAD.models / anomalib.models instead of the broken
  `SARIAD.models.<name>` path and `.lower()` module names (which failed
  for EfficientAd, ReverseDistillation, ...).
- Add `--dry-run`, which validates a config against the real constructor
  signatures without downloading data. It catches the invalid
  `input_size` for Padim and `h` for NLM that default.yaml contained.
- SARIAD.models is lazy, so importing it no longer needs the SARATR-X
  submodule.
- demo.py takes the config path from argv.

Fixes #15

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…port

Findings from running the model against the real SARATR-X checkpoint:

- The pretrained checkpoint contains the encoder only. The model was built
  with the default HiViT config (rpe=True, 8 decoder blocks) instead of the
  one the checkpoint was trained with (mae_hivit_base_dec512d6b), leaving
  136 keys (relative-position tables, decoder blocks) randomly initialized
  and silently ignored (strict=False). Build the matching config and fail
  loudly if any *encoder* weight is missing; the decoder is expected to be
  trained with fit().
- The anomaly map compared the raw 1-channel image to the decoder output,
  but the decoder predicts per-patch-normalized multi-scale SAR gradient
  features (3 x 16 x 16 per patch), not pixels, and the unpatching assumed
  the wrong channel layout. Score with the true feature targets instead,
  reusing the MAE's own target construction (verified equal to the MAE loss
  for a fixed mask), averaging over several masks so all patches are
  covered (mask_ratio was also never passed to the model).
- "Image must have 3 channels": anomalib's Batch only accepts 3 channels, so
  the Grayscale(1) pre-processor could not work. Keep 3 identical channels
  in the batch and reduce to one inside the model.
- Use automatic optimization (AdamW with the layer-wise LR decay groups)
  instead of the manual loop with a CUDA-only GradScaler and a hard-coded
  accelerator, so it runs on CPU and with any Trainer precision. Add
  freeze_encoder (train only the decoder, 21M vs 87M parameters).
- Cache the checkpoint in anomalib's pretrained-weights dir instead of the
  current directory, load with weights_only=True, and report ONE_CLASS
  learning type.

Fixes #9

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ures

The YOLO integration was a stub that did not even import
(YOLOAnomalyModel.forward had no body, YOLOAnomaly overrode fit() with
`pass`). YOLO is a supervised detector, so use it the way PaDiM uses a
CNN: hook intermediate layers of the (pretrained) Ultralytics model,
fit a multivariate Gaussian to the features of normal images and score
with the Mahalanobis distance.

- SARIAD.models.components: FeatureGaussianModel / GaussianAD, anomalib's
  PadimModel/Padim with a pluggable feature extractor (any module that
  returns a dict of feature maps), keeping Padim's embedding subsampling,
  fit and anomaly map. Reused by MSFA.
- UltralyticsFeatureExtractor: forward hooks on model.model[i], so any
  Ultralytics model works (YOLOv8, YOLO11, YOLO26 checked; RT-DETR and
  FastSAM are loaded with their own classes; single-channel models are
  supported). Model.embed() is not used: it global-average-pools the
  features, which loses the spatial layout an anomaly map needs.
- YOLOAnomaly exported from SARIAD.models and usable from the YAML runner.

Fixes #32

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Port of https://github.com/Advanced-Vision-and-Learning-Lab/PaDiM-ACE
(Ibarra & Peeples, arXiv:2504.08049) on top of anomalib 2.6.2. Instead
of copying the fork of anomalib's PaDiM, subclass PadimModel so it keeps
following anomalib.

PaDiM-ACE fits PaDiM's per-patch Gaussian on normal images; the target
signature of each patch position is the mean embedding of anomalous
training images (signature_dir), and a test patch is scored with the
cosine between its whitened embedding and the whitened signature.

Differences from the reference implementation:
- No CUDA hard-coding, no directory layout inferred from image paths, no
  ImageFolder at a fixed 256x256: the signature images are loaded from
  signature_dir through the model's own pre-processor.
- The whitening is computed once after fit (not an SVD per batch) and
  the signatures are stored in the checkpoint.
- Reports ONE_CLASS: the Engine skips training for FEW_SHOT models.

The reference decomposes the *inverse* covariance and applies D^-1/2 U^T
to it, which does not whiten the data (background covariance after the
transform has variances from 0.04 to 335 on a test case, vs identity
for the textbook Sigma^-1/2). `whitening="reference"` (default)
reproduces the original computation (matches ACELoss to 1e-6, verified
against a copy of it); `whitening="standard"` is textbook ACE. Which
one the paper's numbers correspond to should be confirmed by the
authors.

Refs: user request

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…0K method)

The MFSA package was empty. MSFA (Multi-Stage with Filter Augmentation,
SARDet-100K, NeurIPS 2024) is an MMDetection pretraining framework for
SAR object detection, not a standalone network, and its filter code
depends on mmdet and CUDA-only ops, so it cannot be imported as is.

Integrate what applies to anomaly detection, its data-input stage:
stack the raw intensity with SAR filter responses and feed them to a
backbone whose first convolution is widened accordingly (timm adapts the
pretrained weights), then model the features of normal images with the
shared per-patch Gaussian (PaDiM) base.

- filters.py: dependency-free, device-agnostic raw / grad_edge
  (log-ratio-of-averages) / hog / canny filters, unit-checked on
  synthetic edges. The wavelet-scattering and Haar filters of the
  original need extra packages and are not included.
- MSFA(GaussianAD) exported from SARIAD.models, usable from the YAML
  runner. Pre-processing only resizes (the log-ratio needs raw
  intensities, not mean/std-normalized images).

The multi-stage detector pretraining and model-migration stages are not
reproduced (they train MMDetection detectors); SAR-pretrained backbone
weights can be supplied through `backbone` instead. The package
directory keeps the historical MFSA spelling (moving the SARDet_100K
submodule failed on this filesystem); the public class is MSFA, the
upstream name. The IEEE reference in the issue (10433508) could not be
retrieved, so the integration follows the SARDet-100K code.

Fixes #28
Fixes #39

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…odules

#8: The normal-image generation was duplicated inside the MSTAR and
SSDD datamodules. Extract it to SARIAD.utils.normal_gen with a
pluggable segmentation backend (classical KMeans; an experimental
BiRefNet backend behind a lazy import) and two fill methods (random
background pixels, background patch inpainting).
- Verified against the original code: segmentation and blur_mask give
  identical masks; the vectorized background fill draws from the same
  distribution and is ~20x faster than the per-pixel Python loop.
- Fix MSTAR ignoring its own `target_filter` (the filtered list was
  computed and then unused).
- remove_target() (MSTAR's target + shadow recipe) refuses to write a
  "normal" image when the segmentation covers most of the chip. Run on
  real SAMPLE chips, the recipe selects 100% of dB-scaled images, which
  would have produced "normal" images that still contain the target.
- BiRefNet was not run (it needs a ~1 GB download and executes remote
  code); how well an RGB salient-object model transfers to SAR is untested.

#4 SAMPLE_PUBLIC: was a copy-paste stub with empty directories. Generate
{train,test}/{anom,norm,masks} from the PNG chips, target = anomaly, and
split by azimuth (test = highest azimuths) so near-identical neighbouring
aspect angles don't leak across the split. Defaults to qpm scaling, where
the segmentation works (checked visually on real chips).

#2 SARDet_100K: was a stub (class named SAMPLE_PUBLIC, empty dirs, wrong
download call). Build the benchmark from the COCO annotations: annotated
objects are the anomalies (box masks), normal images are the boxes filled
with clutter. Category filter and per-split image cap, because filling
large images is slow.

Both new datamodules are exported and accept path/num_workers/Folder
arguments like the others. The SARDet-100K download (Kaggle, tens of GB)
was not run; its generation was tested on a synthetic COCO fixture in the
Kaggle folder layout. SAMPLE was tested on 11 real chips downloaded
individually, not the full 1.5 GB archive.

Fixes #2
Fixes #4
Fixes #8

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
utils/inf.py only had a post-hoc `Metrics` class. Add `Inferencer`, a
stateful evaluator built on torchmetrics (MetricCollection, update /
compute / reset, optional binned pixel AUROC for constant memory) that
can also run a model over a datamodule (`evaluate`). `Metrics` now
delegates to it, so the reporting and the inferencer share one
implementation and predictions are streamed instead of concatenating
every mask.

Fixes in the metrics themselves:
- "AUROC (Pixel-level)" was computed from the *binarized* predicted
  mask, which is not an AUROC. Use the continuous anomaly map (checked
  against scikit-learn to 6 decimals). All other metrics are identical to
  the previous implementation (compared on noisy predictions with a
  non-trivial confusion matrix).
- AUROC on data with a single class now reports "N/A" instead of a
  misleading 0.0.
- Invalid `\pm` escape sequences in non-raw strings (SyntaxWarning),
  `mkdir` without parents, the hard-coded "comparison_output" folder
  (the runner now writes the ROC comparison next to the comparison
  table), and the stale print-based example().

Fixes #17

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There were no tests. Add a pytest suite that runs offline on CPU
(nothing downloads weights or datasets, no git submodule is required):

- imports (must not need a GPU/submodules), config resolution and
  --dry-run validation, archive path-traversal protection
- normal-data generation, including the guard that refuses to write a
  "normal" image when the target segmentation covers most of the chip
- metrics: Inferencer/Metrics against scikit-learn, streaming vs single
  batch, single-class and missing-mask reporting, outputs
- models: shared Gaussian base, PaDiM-ACE against a copy of the original
  scoring (oracle), whitening properties, checkpoint round trip, MSFA
  filters/model, SARATRX unpatch layout and map aggregation
- dataset generation for SAMPLE (azimuth split) and SARDet-100K (COCO,
  Kaggle-style nesting)
- end to end runner -> Engine -> metrics runs on a synthetic dataset for
  Padim, PaDiM-ACE and MSFA
- SAR-CNN lazy loading and the log-domain helpers

Tests that need downloads or checkpoints (YOLO weights, the SARATR-X
checkpoint) are gated behind SARIAD_TEST_NETWORK /
SARIAD_TEST_SARATRX_CHECKPOINT. Sabotaging the ACE whitening makes the
suite fail, i.e. the tests are not vacuous.

CI: run pytest on every push and pull request; the build/publish jobs
depend on it (publishing remains tag-only).

Refs #14

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…#14)

- Delete SARIAD/main.py: it imported modules that do not exist
  (`models`, `benchmarks.benchmarking`, `SARDataModule`) and expected a
  config schema nothing produces. The real entry point is
  SARIAD.config.run:main, now also installed as the `sariad` command.
- Delete the unreferenced legacy models (anomalib_models.py,
  autoencoder.py, transformer.py): they used pytorch_lightning, assumed
  (x, y) tuple batches and a fixed 256x256 input, and could not be
  trained through anomalib's Engine.
- Replace print() in the library code (downloads, dataset generation)
  with logging; the runner and demo enable log output.
- Remove unused imports and placeholder-less f-strings; add the missing
  __init__.py files (Default/, ssdd/); drop a commented-out demo block.
- demo/train.py was a script of commented-out alternatives: make it a
  small CLI that picks dataset/model/preprocessor by name.

Refs #14

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Refs #14

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- docs/conf.py computed the project root one level too high, so autodoc
  could not import SARIAD; index.md was a README copy with a broken
  figure path and no toctree; the API pages documented the deleted
  SARIAD.main. Fix conf.py, add a real index with a toctree, drop the
  stale requirements.txt (Read the Docs now installs the `docs` extra on
  Python 3.13 with CPU PyTorch and the submodules), untrack the 216
  committed docs/_build files.
- Dataset and model tables are generated from metadata that lives next to
  the code (DATASET_INFO in each datamodule, MODELS_INFO in
  SARIAD.models; docs/gen_docs.py runs from conf.py), so they cannot
  drift. A test checks the metadata covers every exported class.
- docs/datasets.md: how normal data is generated and its known
  limitation, the train/test conventions (including a note on MSTAR/SSDD),
  and candidate datasets whose links were checked (#5).
- docs/models.md: SARIAD models, pre-processing, and the literature review
  of the candidates in #43 with license and last-push facts read from
  GitHub (#27, #43). Nothing listed there was run.
- README: the directory tree, usage and "benchmark.enabled" it described
  did not exist. Rewrite it around what is actually there.

The docs build was not run (the environment was lost); see HANDOFF.md.
figs/overall.svg is untouched: its component lists are out of date.

Fixes #7
Fixes #5
Fixes #27
Fixes #43
Fixes #14
Refs #19

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
What was done per issue, what was and was not verified (nothing was run
on real SAR data or a GPU), open items in priority order, the SARATRX and
PaDiM-ACE findings that need a decision, design decisions, and the
WSL/NTFS environment traps hit along the way.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The docs now build with `sphinx-build -W --keep-going`:
- regenerate docs/source/*.rst with sphinx-apidoc (new datamodule/model modules)
- generated dataset details use bold labels instead of headings (myst header level warning)
- autodoc_inherit_docstrings=False: the torchvision Transform.transform docstring
  links to a label that only exists in torchvision's docs
- blob_utils docstrings in Google style, MSFA filter table column widened

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
`submodules: recursive` fails on the SARATR-X fork: its nested submodule
detection/apex has no URL in .gitmodules, so the Test job never got past
checkout. The code only needs the top-level submodules.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Even with `submodules: true`, actions/checkout runs `git submodule foreach --recursive`
to manage submodule credentials. That fails on the SARATR-X fork, whose nested
detection/apex submodule has no URL in .gitmodules, so the Test job never got past
checkout. A plain non-recursive `git submodule update --init` works.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@jpeeples67
jpeeples67 merged commit 9905e97 into main Sep 20, 2026
6 checks passed
This was referenced Sep 20, 2026
jpeeples67 added a commit that referenced this pull request Sep 20, 2026
- HANDOFF.md: PR #47 is merged, issue states (#9/#28/#39 open on purpose), GPU
  verification numbers for SARATRX, the CI submodule finding, Windows gotchas,
  and the updated open items.
- tests/conftest.py: set MPLBACKEND=Agg. test_runner_end_to_end failed
  intermittently on Windows with a Tk TclError (init.tcl) because matplotlib
  defaulted to the Tk backend for figures that are only saved to files.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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.

1 participant