[Bug]: Tricky behavior when config.project.seed is set to 0

[tanemaki]

Describe the bug

Hi,

Thanks for the great work :) I have just encountered unexpected behavior in seed specification, so here is the issue description.

In the current implementation, when config.project.seed is set to 0, a seed is not fixed to 0 but randomized, as shown in the following code:

anomalib/tools/train.py

Lines 50 to 51 in 764f97d

	if config.project.seed:
	seed_everything(config.project.seed)

anomalib/tools/hpo/sweep.py

Lines 38 to 39 in 764f97d

	if model_config.project.seed != 0:
	seed_everything(model_config.project.seed)

I feel that the seed should be randomized either

• when there is no seed key under config.project, or
• when config.project.seed is explicitly set to None.

So how about rewriting the above code as follows?

if config.project.get("seed") is not None:
    seed_everything(config.project.seed)

If this change doesn't bother somebody who already uses 0 to randomize seeds, I will create a pull request.

What do you think?

Dataset

N/A

Model

N/A

Steps to reproduce the behavior

1. Run python tools/train.py --model patchcore

The seed is set to 0 in patchcore's config.yaml, so the seed is randomized.

anomalib/anomalib/models/patchcore/config.yaml

Line 54 in 764f97d

seed: 0

OS information

OS information:

• OS: macOS Monterey
• Python version: 3.8.13
• Anomalib version: 0.3.7
• PyTorch version: 1.11.0
• CUDA/cuDNN version: N/A
• GPU models and configuration: N/A
• Any other relevant information: N/A

Expected behavior

When a seed is set to 0, the seed should be fixed to 0 instead of using a random seed.

Screenshots

No response

Pip/GitHub

GitHub

What version/branch did you use?

No response

Configuration YAML

dataset:
  name: mvtec #options: [mvtec, btech, folder]
  format: mvtec
  path: ./datasets/MVTec
  task: segmentation
  category: bottle
  image_size: 224
  train_batch_size: 32
  test_batch_size: 1
  num_workers: 8
  transform_config:
    train: null
    val: null
  create_validation_set: false
  tiling:
    apply: false
    tile_size: null
    stride: null
    remove_border_count: 0
    use_random_tiling: False
    random_tile_count: 16

model:
  name: patchcore
  backbone: wide_resnet50_2
  pre_trained: true
  layers:
    - layer2
    - layer3
  coreset_sampling_ratio: 0.1
  num_neighbors: 9
  normalization_method: min_max # options: [null, min_max, cdf]

metrics:
  image:
    - F1Score
    - AUROC
  pixel:
    - F1Score
    - AUROC
  threshold:
    method: adaptive #options: [adaptive, manual]
    manual_image: null
    manual_pixel: null

visualization:
  show_images: False # show images on the screen
  save_images: True # save images to the file system
  log_images: True # log images to the available loggers (if any)
  image_save_path: null # path to which images will be saved
  mode: full # options: ["full", "simple"]

project:
  seed: 0
  path: ./results

logging:
  logger: [] # options: [comet, tensorboard, wandb, csv] or combinations.
  log_graph: false # Logs the model graph to respective logger.

optimization:
  export_mode: null # options: onnx, openvino

# PL Trainer Args. Don't add extra parameter here.
trainer:
  accelerator: auto # <"cpu", "gpu", "tpu", "ipu", "hpu", "auto">
  accumulate_grad_batches: 1
  amp_backend: native
  auto_lr_find: false
  auto_scale_batch_size: false
  auto_select_gpus: false
  benchmark: false
  check_val_every_n_epoch: 1 # Don't validate before extracting features.
  default_root_dir: null
  detect_anomaly: false
  deterministic: false
  devices: 1
  enable_checkpointing: true
  enable_model_summary: true
  enable_progress_bar: true
  fast_dev_run: false
  gpus: null # Set automatically
  gradient_clip_val: 0
  ipus: null
  limit_predict_batches: 1.0
  limit_test_batches: 1.0
  limit_train_batches: 1.0
  limit_val_batches: 1.0
  log_every_n_steps: 50
  log_gpu_memory: null
  max_epochs: 1
  max_steps: -1
  max_time: null
  min_epochs: null
  min_steps: null
  move_metrics_to_cpu: false
  multiple_trainloader_mode: max_size_cycle
  num_nodes: 1
  num_processes: null
  num_sanity_val_steps: 0
  overfit_batches: 0.0
  plugins: null
  precision: 32
  profiler: null
  reload_dataloaders_every_n_epochs: 0
  replace_sampler_ddp: true
  strategy: null
  sync_batchnorm: false
  tpu_cores: null
  track_grad_norm: -1
  val_check_interval: 1.0 # Don't validate before extracting features.

Logs

N/A

Code of Conduct

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

[djdameln]

We agree with your observations, and your proposed solution sounds good. It would be great if you're willing to submit a PR for this and become a contributor.

The PR could include a warning message if the value of the seed is zero, explaining that the behavior has changed for this seed value. I think the most appropriate place for this warning message would be inside the get_configurable_parameters function, which is responsible for parsing the config.yaml.

[tanemaki]

Thanks, then I will submit a pull request with a modified code and a warning message.

But it seems the branch named origin/da/fix-zero-seed already exists. Is it OK for me to work on it?

[tanemaki]

@djdameln It seems I don't have permission to push my branch to create a pull request.

[samet-akcay]

@tanemaki, you'll probably need to create a fork, from which you could create the pull request.

[tanemaki]

@samet-akcay Oh, I see! Thanks for the comment :)

Ah, it is stated in the contributing guideline, which I completely forgot to read in the first place. It's my bad.
https://github.com/openvinotoolkit/anomalib/blob/main/CONTRIBUTING.md

[samet-akcay]

@tanemaki, no problem at all

[tanemaki]

Because I could not pass the test on my environment, I have created a draft pull request to share my code change :)

#766

[tanemaki]

I have created a fork repository, but it seems the test (tox -e pre-commit) has failed even if I have used the unmodified main branch. I am using macOS (12.6) on Intel Mac. Do I need to use the docker to pass the test?

Here are the steps to produce test failure.

1. Pull a fork repository to my local machine
2. conda create -n forked-anomalib-toxenv python=3.8
3. conda activate forked-anomalib-toxenv
4. pip install tox
5. tox -e pre-commit

will produce the following output.

-> % tox -e pre-commit
pre-commit inst-nodeps: /Users/tanemaki/git/forked-anomalib/.tox/.tmp/package/1/anomalib-0.3.7.tar.gz
pre-commit installed: anomalib @ file:///Users/tanemaki/git/forked-anomalib/.tox/.tmp/package/1/anomalib-0.3.7.tar.gz,cfgv==3.3.1,distlib==0.3.6,filelock==3.8.2,identify==2.5.9,nodeenv==1.7.0,platformdirs==2.5.4,pre-commit==2.20.0,PyYAML==6.0,toml==0.10.2,virtualenv==20.17.1
pre-commit run-test-pre: PYTHONHASHSEED='2620866531'
pre-commit run-test: commands[0] | pre-commit run --all-files
trim trailing whitespace.................................................Passed
fix end of files.........................................................Passed
check yaml...............................................................Passed
check for added large files..............................................Passed
debug statements (python)................................................Passed
detect private key.......................................................Passed
black....................................................................Passed
isort....................................................................Passed
flake8...................................................................Passed
pylint...................................................................Failed
- hook id: pylint
- exit code: 6

************* Module anomalib.utils.loggers.base
anomalib/utils/loggers/base.py:6:0: E0611: No name 'abstractmethod' in module 'abc' (no-name-in-module)
************* Module anomalib.utils.callbacks
anomalib/utils/callbacks/__init__.py:63:16: E1101: Module 'os' has no 'path' member (no-member)
anomalib/utils/callbacks/__init__.py:123:31: E1101: Module 'os' has no 'path' member (no-member)
************* Module anomalib.utils.cli.cli
anomalib/utils/cli/cli.py:48:4: W0235: Useless super delegation in method '__init__' (useless-super-delegation)
anomalib/utils/cli/cli.py:155:31: E1101: Module 'os' has no 'path' member (no-member)
anomalib/utils/cli/cli.py:193:20: E1101: Module 'os' has no 'path' member (no-member)
anomalib/utils/cli/cli.py:236:28: E1101: Module 'os' has no 'path' member (no-member)
anomalib/utils/cli/cli.py:244:15: E1101: Module 'os' has no 'path' member (no-member)
anomalib/utils/cli/cli.py:250:32: E1101: Module 'os' has no 'path' member (no-member)
************* Module anomalib.utils.callbacks.nncf.callback
anomalib/utils/callbacks/nncf/callback.py:87:8: E1101: Module 'os' has no 'makedirs' member (no-member)
anomalib/utils/callbacks/nncf/callback.py:88:20: E1101: Module 'os' has no 'path' member (no-member)
anomalib/utils/callbacks/nncf/callback.py:91:8: E1101: Module 'os' has no 'system' member (no-member)
************* Module anomalib.deploy.inferencers.torch_inferencer
anomalib/deploy/inferencers/torch_inferencer.py:36:4: W0231: __init__ method from base class 'Inferencer' is not called (super-init-not-called)
************* Module anomalib.deploy.inferencers.openvino_inferencer
anomalib/deploy/inferencers/openvino_inferencer.py:35:4: W0231: __init__ method from base class 'Inferencer' is not called (super-init-not-called)
************* Module anomalib.pre_processing.tiler
anomalib/pre_processing/tiler.py:194:13: W1116: Second argument of isinstance is not a type (isinstance-second-argument-not-valid-type)
************* Module anomalib.models.components.base.dynamic_module
anomalib/models/components/base/dynamic_module.py:6:0: E0611: No name 'ABC' in module 'abc' (no-name-in-module)
************* Module anomalib.utils.callbacks.visualizer.visualizer_base
anomalib/utils/callbacks/visualizer/visualizer_base.py:86:47: W0613: Unused argument 'pl_module' (unused-argument)
************* Module anomalib.data.utils.download
anomalib/data/utils/download.py:115:29: E1101: Module 'io' has no 'TextIOWrapper' member (no-member)
anomalib/data/utils/download.py:115:47: E1101: Module 'io' has no 'StringIO' member (no-member)
************* Module anomalib.models.components.base.anomaly_module
anomalib/models/components/base/anomaly_module.py:7:0: E0611: No name 'ABC' in module 'abc' (no-name-in-module)
************* Module benchmarking.benchmark
tools/benchmarking/benchmark.py:65:27: E1101: Module 'io' has no 'StringIO' member (no-member)
************* Module anomalib.deploy.inferencers.base_inferencer
anomalib/deploy/inferencers/base_inferencer.py:6:0: E0611: No name 'ABC' in module 'abc' (no-name-in-module)
anomalib/deploy/inferencers/base_inferencer.py:6:0: E0611: No name 'abstractmethod' in module 'abc' (no-name-in-module)
************* Module anomalib.utils.callbacks.export
anomalib/utils/callbacks/export.py:44:8: E1101: Module 'os' has no 'makedirs' member (no-member)
************* Module anomalib.utils.callbacks.tiler_configuration
anomalib/utils/callbacks/tiler_configuration.py:54:73: W0613: Unused argument 'stage' (unused-argument)
************* Module anomalib.models
anomalib/models/__init__.py:96:35: E1101: Module 'os' has no 'path' member (no-member)
************* Module anomalib.utils.sweep.config
anomalib/utils/sweep/config.py:8:0: E0611: No name 'Iterable' in module 'collections.abc' (no-name-in-module)
anomalib/utils/sweep/config.py:8:0: E0611: No name 'ValuesView' in module 'collections.abc' (no-name-in-module)
************* Module anomalib.utils.loggers
anomalib/utils/loggers/__init__.py:100:29: E1101: Module 'os' has no 'path' member (no-member)
anomalib/utils/loggers/__init__.py:105:27: E1101: Module 'os' has no 'path' member (no-member)
anomalib/utils/loggers/__init__.py:106:12: E1101: Module 'os' has no 'makedirs' member (no-member)
anomalib/utils/loggers/__init__.py:120:27: E1101: Module 'os' has no 'path' member (no-member)
anomalib/utils/loggers/__init__.py:121:12: E1101: Module 'os' has no 'makedirs' member (no-member)
anomalib/utils/loggers/__init__.py:131:50: E1101: Module 'os' has no 'path' member (no-member)

mypy.....................................................................Passed
pydocstyle...............................................................Passed
nbqa-black...............................................................Passed
nbqa-isort...............................................................Passed
nbqa-flake8..............................................................Passed
nbqa-pylint..............................................................Passed
prettier.................................................................Passed
markdownlint.............................................................Passed
Lint Dockerfiles.........................................................Passed
ERROR: InvocationError for command /Users/tanemaki/git/forked-anomalib/.tox/pre-commit/bin/pre-commit run --all-files (exited with code 1)
_____________________________________________________________________________________ summary _____________________________________________________________________________________
ERROR:   pre-commit: commands failed

[samet-akcay]

@tanemaki, the PR seems to pass in our CI tests.

[tanemaki]

@samet-akcay Oh, really? Thanks for letting me know. Then, do I need to press "Ready for review" button to convert the draft PR to the formal PR?

[samet-akcay]

Yes please