[Bug]: anomaly map normalization in efficient ad looks wrong

[jpcbertoldo]

Describe the bug

Poiting out a few things:

• A) it looks like the val split in the current yaml config of efficientad is cheating : )
• B) in its lightning module, the validation phase does the whole job in the start then behaves like a test phase
• C) the set of images used for the penalty in $L_{\text{ST}}$ is a small sample of imagenet

A) normalization params should depend on a split from the train set

The default YAML config is using a random split from the test set

anomalib/src/anomalib/models/efficient_ad/config.yaml

Line 18 in c97762c

val_split_mode: same_as_test # options: [same_as_test, from_test, synthetic]

which is usual for models that only do evaluation on the val set, but in efficientad

(from http://arxiv.org/abs/2303.14535, Section 3.4, page 6, left column)

☝️ the validation set is like a 2nd-phase (or 3rd counting the distillation) of the training, where $q^{\text{ST}}_a, q^{\text{ST}}_b, q^{\text{AE}}_a, q^{\text{AE}}_b$ are fitted.

It should be possible to split the train set and use it there.

---

B) validation start/step design is weird

I think lightning's design is made essentially to avoid loops on the batches.
By implementing validation_step(batch) you define what the model should do with that data -- which, for EfficientAD, is to find the $q$ parameters. In the current design, there is a for loop in the start

anomalib/src/anomalib/models/efficient_ad/lightning_model.py

Lines 245 to 252 in c97762c

	def on_validation_start(self) -> None:
	"""
	Calculate the feature map quantiles of the validation dataset and push to the model.
	"""
	if (self.current_epoch + 1) == self.trainer.max_epochs:
	map_norm_quantiles = self.map_norm_quantiles(self.trainer.datamodule.val_dataloader())
	self.model.quantiles.update(map_norm_quantiles)

function map_norm_quantiles()

anomalib/src/anomalib/models/efficient_ad/lightning_model.py

Lines 154 to 179 in c97762c

	@torch.no_grad()
	def map_norm_quantiles(self, dataloader: DataLoader) -> dict[str, Tensor]:
	"""Calculate 90% and 99.5% quantiles of the student(st) and autoencoder(ae).
	Args:
	dataloader (DataLoader): Dataloader of the respective dataset.
	Returns:
	dict[str, Tensor]: Dictionary of both the 90% and 99.5% quantiles
	of both the student and autoencoder feature maps.
	"""
	maps_st = []
	maps_ae = []
	logger.info("Calculate Validation Dataset Quantiles")
	for batch in tqdm.tqdm(dataloader, desc="Calculate Validation Dataset Quantiles", position=0, leave=True):
	for img, label in zip(batch["image"], batch["label"]):
	if label == 0: # only use good images of validation set!
	output = self.model(img.to(self.device))
	map_st = output["map_st"]
	map_ae = output["map_ae"]
	maps_st.append(map_st)
	maps_ae.append(map_ae)
	qa_st, qb_st = self._get_quantiles_of_maps(maps_st)
	qa_ae, qb_ae = self._get_quantiles_of_maps(maps_ae)
	return {"qa_st": qa_st, "qa_ae": qa_ae, "qb_st": qb_st, "qb_ae": qb_ae}

which is re-doing this loop-over-batches structure, while validation_step() is just predicting (which would happen in the test_step() or predict_step()):

anomalib/src/anomalib/models/efficient_ad/lightning_model.py

Lines 253 to 266 in c97762c

	def validation_step(self, batch: dict[str, str | Tensor], *args, **kwargs) -> STEP_OUTPUT:
	"""Validation Step of EfficientAd returns anomaly maps for the input image batch
	Args:
	batch (dict[str, str | Tensor]): Input batch
	Returns:
	Dictionary containing anomaly maps.
	"""
	del args, kwargs # These variables are not used.
	batch["anomaly_maps"] = self.model(batch["image"])["anomaly_map_combined"]
	return batch

Side note: is it a good idea to have tqdm inside the model?

---

C) small imagenet

The data used in the penalty term of the student's loss function should be the same as the one from the teacher distillation (imagenet in the paper).

The code is currently downloading a reduced version with 10 classes.

The user should be able to use a custom folder (with imagenet already setup).

Code of Conduct

• I agree to follow this project's Code of Conduct

[alexriedel1]

Hey!
Thanks for adressing these points, let me clear some of them :)

A) it looks like the val split in the current yaml config of efficientad is cheating : )

Yes you're right about that. Fixing this isn't too easy in anomalib I guess, so the question is how this is affecting the model performance and if it is a real problem. Of course you shouldn't show the model anything of the validation set during training, but for this kind of normalization the model might not learn to much information from the validation data.

B) in its lightning module, the validation phase does the whole job in the start then behaves like a test phase

I don't get this completely. Why shouldn't it be designed this way? Using the tqdm in the model however seems fine to me, as long as its not in the forward function of the torch model (that is used for inference in the end)

C) the set of images used for the penalty in is a small sample of imagenet

Thats right, however the original paper also only uses a subset of 60k imagenet images..
again the question is, how does this affect the models performance.

[jpcbertoldo]

A) it looks like the val split in the current yaml config of efficientad is cheating : )

Yes you're right about that. Fixing this isn't too easy in anomalib I guess, so the question is how this is affecting the model performance and if it is a real problem. Of course you shouldn't show the model anything of the validation set during training, but for this kind of normalization the model might not learn to much information from the validation data.

I corrected that on my fork (will open the PR soon).
I am running a benchmark and the results are very disappointing (there's an visa class with AUPRO 0 🙃 ) so I'm wondering if this is coming from there -- I also had other two minor changes, so will roll them back before o check.

B) in its lightning module, the validation phase does the whole job in the start then behaves like a test phase

I don't get this completely. Why shouldn't it be designed this way? Using the tqdm in the model however seems fine to me, as long as its not in the forward function of the torch model (that is used for inference in the end)

I think lightning's design is made essentially to avoid loops on the batches.

By implementing validation_step(batch) you define what the model should do with that data -- which, for EfficientAD, is to find the $q$ parameters.

In the current design, there is a for loop in the start

anomalib/src/anomalib/models/efficient_ad/lightning_model.py

Line 168 in c97762c

for batch in tqdm.tqdm(dataloader, desc="Calculate Validation Dataset Quantiles", position=0, leave=True):

which is re-doing this structure, while validation_step is really just predicting (which would happen in the test_step() or predict_step().

C) the set of images used for the penalty in is a small sample of imagenet

Thats right, however the original paper also only uses a subset of 60k imagenet images.. again the question is, how does this affect the models performance.

The 60k you refer to is from here, right?

(appendix A.2, algorithm 3, page 16)

Well, using the imagenet1k (1k classes, 1.2M images), would be quite more diverse then this subset of only 10 classes, but, yes, effect to be tested.

The paper's ablation suggests a somewhat important effect

This is one of the things I changed and will be rolling back to see the effect.

[jpcbertoldo]

I adressed A and C in #1376