🚀 Allow passing model to engine constructor #1780

danylo-boiko · 2024-02-27T18:57:20Z

📝 Description

Addresses [Task]: Allow passing model to engine constructor #1639

✨ Changes

Select what type of change your PR is:

🐞 Bug fix (non-breaking change which fixes an issue)
🔨 Refactor (non-breaking change which refactors the code base)
🚀 New feature (non-breaking change which adds functionality)
💥 Breaking change (fix or feature that would cause existing functionality to not work as expected)
📚 Documentation update
🔒 Security update

✅ Checklist

Before you submit your pull request, please make sure you have completed the following steps:

📋 I have summarized my changes in the CHANGELOG and followed the guidelines for my type of change (skip for minor changes, documentation updates, and test enhancements).
📚 I have made the necessary updates to the documentation (if applicable).
🧪 I have written tests that support my changes and prove that my fix is effective or my feature works (if applicable).

For more information about code review checklists, see the Code Review Checklist.

Signed-off-by: Danylo Boiko <danielboyko02@gmail.com>

…-to-engine-constructor

Signed-off-by: Danylo Boiko <danielboyko02@gmail.com>

danylo-boiko · 2024-02-28T12:44:04Z

Hi @samet-akcay. Could you please run the unit tests? If all goes well, I will update the codebase and documentation to use the new API in a few days.

ashwinvaidya17 · 2024-02-28T14:14:09Z

@danylo-boiko the tests are triggered only for non-draft PRs. You can either mark this as ready for review or run the tests locally

Signed-off-by: Danylo Boiko <danielboyko02@gmail.com>

review-notebook-app · 2024-02-28T22:18:50Z

Check out this pull request on

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

Signed-off-by: Danylo Boiko <danielboyko02@gmail.com>

ashwinvaidya17

Thanks for the efforts!

Signed-off-by: Danylo Boiko <danielboyko02@gmail.com>

samet-akcay · 2024-03-18T14:33:02Z

README.md

 datamodule = MVTec()
-model = Patchcore()
-engine = Engine()
+engine = Engine(model=Patchcore())


I think this change should not change the default behaviour, so this document could stay as is. If we want to mention this feature, this could be an additional example.

samet-akcay

Thanks for creating this PR. I've got some comments, but would also like to hear @djdameln's opinions.

samet-akcay · 2024-03-18T14:33:36Z

docs/source/markdown/guides/how_to/models/feature_extractors.md

@@ -103,10 +103,10 @@ from anomalib.engine import Engine
 datamodule = MVTec(num_workers=0)
 # Specify backbone and layers
 model = Padim(backbone="resnet18", layers=["layer1", "layer2"])
-engine = Engine(image_metrics=["AUROC"], pixel_metrics=["AUROC"])
+engine = Engine(model=model, image_metrics=["AUROC"], pixel_metrics=["AUROC"])


Same here, I think. The default behaviour should stay the same.

samet-akcay · 2024-03-18T14:39:26Z

src/anomalib/engine/engine.py

@@ -155,8 +156,12 @@ def __init__(
        if self.task == TaskType.SEGMENTATION:
            self.pixel_metric_names = pixel_metrics if pixel_metrics is not None else ["AUROC", "F1Score"]

+        self._model: AnomalyModule | None = None


What is the difference between self.model and self._model?

self._model is a field. I created a getter/setter self.model to move the self._setup_trainer(model) call to the setter and keep the logic simpler.

samet-akcay · 2024-03-18T14:42:18Z

src/anomalib/engine/engine.py

@@ -495,24 +511,33 @@ def fit(
                anomalib fit --config <config_file_path>
                ```
        """
+        if model:
+            self.model = model
+        elif not self.model:


Suggested change

elif not self.model:

if not self.model:

Why do we need this double check?

samet-akcay · 2024-03-18T14:46:03Z

src/anomalib/engine/engine.py

+            >>> engine = Engine(model=model)
+            >>> engine.fit(datamodule=datamodule)
+            >>> engine.test(datamodule=datamodule)


Same as above, this should not change the default behaviour. We could provide an additional example, showing this new feature.

Signed-off-by: Danylo Boiko <danielboyko02@gmail.com>

danylo-boiko · 2024-03-18T15:27:52Z

Thanks for creating this PR. I've got some comments, but would also like to hear @djdameln's opinions.

Thank you for your feedback, I have made the necessary changes.

Signed-off-by: Danylo Boiko <danielboyko02@gmail.com>

danylo-boiko · 2024-04-15T07:35:55Z

@samet-akcay, could you please take a look at the changes/comments when you have some free time?

danylo-boiko and others added 6 commits February 27, 2024 20:53

Add a model parameter to the engine constructor

6cefe62

Signed-off-by: Danylo Boiko <danielboyko02@gmail.com>

Merge remote-tracking branch 'upstream/main' into allow-passing-model…

115ad3b

…-to-engine-constructor

Change model expected type for the export method

613c94f

Signed-off-by: Danylo Boiko <danielboyko02@gmail.com>

Fix the issue with the in-memory model field

53c25c1

Signed-off-by: Danylo Boiko <danielboyko02@gmail.com>

Refactor getter engine.model

2a83a42

Signed-off-by: Danylo Boiko <danielboyko02@gmail.com>

Merge branch 'main' into allow-passing-model-to-engine-constructor

69b3dbb

ashwinvaidya17 requested review from ashwinvaidya17 and djdameln February 28, 2024 14:14

danylo-boiko and others added 3 commits February 28, 2024 18:11

Merge branch 'main' into allow-passing-model-to-engine-constructor

68daf67

Merge branch 'main' into allow-passing-model-to-engine-constructor

b7c9464

Update the usage of the model parameter for tests and tools

bdeec2b

Signed-off-by: Danylo Boiko <danielboyko02@gmail.com>

github-actions bot added Inference Tests Tools labels Feb 28, 2024

Update the usage of the model parameter for docs

8e6a2b1

Signed-off-by: Danylo Boiko <danielboyko02@gmail.com>

github-actions bot added the Docs label Feb 28, 2024

Update the usage of the model parameter for notebooks

31956d6

Signed-off-by: Danylo Boiko <danielboyko02@gmail.com>

github-actions bot added the Notebooks label Feb 28, 2024

danylo-boiko added 4 commits February 29, 2024 00:34

Revert changes for notebook 401

387c790

Signed-off-by: Danylo Boiko <danielboyko02@gmail.com>

Update the usage of the model parameter for src

3a68efa

Signed-off-by: Danylo Boiko <danielboyko02@gmail.com>

Add a log to changelogs

10853bc

Signed-off-by: Danylo Boiko <danielboyko02@gmail.com>

Raname dataset to datamodule in comments

5f2f5db

Signed-off-by: Danylo Boiko <danielboyko02@gmail.com>

danylo-boiko marked this pull request as ready for review February 28, 2024 23:14

danylo-boiko requested review from samet-akcay and paularamo as code owners February 28, 2024 23:14

Add the self._model field for all engines

521c168

Signed-off-by: Danylo Boiko <danielboyko02@gmail.com>

ashwinvaidya17 approved these changes Feb 29, 2024

View reviewed changes

danylo-boiko and others added 4 commits March 6, 2024 22:39

Merge branch 'main' into allow-passing-model-to-engine-constructor

5d22691

Merge branch 'main' into allow-passing-model-to-engine-constructor

1aad0b9

Fix changelog

cf02da7

Signed-off-by: Danylo Boiko <danielboyko02@gmail.com>

Merge branch 'main' into allow-passing-model-to-engine-constructor

0646a0d

samet-akcay reviewed Mar 18, 2024

View reviewed changes

samet-akcay added Enhancement New feature or request Requires Changes Reviewers request changes, which should be addressed by the PR maker Engine and removed Inference Tests Tools Notebooks Docs labels Mar 18, 2024

Merge branch 'main' into allow-passing-model-to-engine-constructor

1a01d19

github-actions bot added Inference Tests Tools Notebooks Docs labels Mar 18, 2024

Add code review changes

0274580

Signed-off-by: Danylo Boiko <danielboyko02@gmail.com>

danylo-boiko requested a review from samet-akcay March 18, 2024 15:25

danylo-boiko and others added 4 commits March 18, 2024 19:10

Merge branch 'main' into allow-passing-model-to-engine-constructor

e947a44

Merge branch 'main' into allow-passing-model-to-engine-constructor

537dd0f

Merge branch 'main' into allow-passing-model-to-engine-constructor

b343c57

Fix using a previously loaded model

cca7b17

Signed-off-by: Danylo Boiko <danielboyko02@gmail.com>

samet-akcay modified the milestones: v1.1.0, v1.2.0 May 24, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🚀 Allow passing model to engine constructor #1780

🚀 Allow passing model to engine constructor #1780

danylo-boiko commented Feb 27, 2024 •

edited

danylo-boiko commented Feb 28, 2024

ashwinvaidya17 commented Feb 28, 2024

review-notebook-app bot commented Feb 28, 2024

ashwinvaidya17 left a comment

samet-akcay Mar 18, 2024

samet-akcay left a comment

samet-akcay Mar 18, 2024

samet-akcay Mar 18, 2024

danylo-boiko Mar 18, 2024

samet-akcay Mar 18, 2024

danylo-boiko Mar 18, 2024

samet-akcay Mar 18, 2024

danylo-boiko commented Mar 18, 2024

danylo-boiko commented Apr 15, 2024

🚀 Allow passing model to engine constructor #1780

Are you sure you want to change the base?

🚀 Allow passing model to engine constructor #1780

Conversation

danylo-boiko commented Feb 27, 2024 • edited

📝 Description

✨ Changes

✅ Checklist

danylo-boiko commented Feb 28, 2024

ashwinvaidya17 commented Feb 28, 2024

review-notebook-app bot commented Feb 28, 2024

ashwinvaidya17 left a comment

Choose a reason for hiding this comment

samet-akcay Mar 18, 2024

Choose a reason for hiding this comment

samet-akcay left a comment

Choose a reason for hiding this comment

samet-akcay Mar 18, 2024

Choose a reason for hiding this comment

samet-akcay Mar 18, 2024

Choose a reason for hiding this comment

danylo-boiko Mar 18, 2024

Choose a reason for hiding this comment

samet-akcay Mar 18, 2024

Choose a reason for hiding this comment

danylo-boiko Mar 18, 2024

Choose a reason for hiding this comment

samet-akcay Mar 18, 2024

Choose a reason for hiding this comment

danylo-boiko commented Mar 18, 2024

danylo-boiko commented Apr 15, 2024

danylo-boiko commented Feb 27, 2024 •

edited