Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MVTec 3D and Folder3D #942

Merged
merged 41 commits into from
Mar 2, 2023
Merged

Conversation

alexriedel1
Copy link
Contributor

Hi,
it's the fix of the failing tests of #907.

The dataloader always expects mask paths even if no mask paths are provided. I removed the dummy insertion of the mask paths and now added it back again.

I think its debatable whether the dataloader should always load the mask paths, but thats another PR I guess:

def __getitem__(self, index: int) -> dict[str, str | Tensor]:
"""Get dataset item for the index ``index``.
Args:
index (int): Index to get the item.
Returns:
Union[dict[str, Tensor], dict[str, str | Tensor]]: Dict of image tensor during training.
Otherwise, Dict containing image path, target path, image tensor, label and transformed bounding box.
"""
image_path = self._samples.iloc[index].image_path
mask_path = self._samples.iloc[index].mask_path
label_index = self._samples.iloc[index].label_index

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@github-actions github-actions bot added Data Docs Notebooks Post-Processing The components that are related to post-processing labels Mar 2, 2023
Comment on lines +103 to +106
(e.g. image: '000.png', mask: '000.png')."
else:
samples["mask_path"] = ""

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thats the actual new part

Copy link
Contributor

@djdameln djdameln left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix. I agree that we should not load the paths when the task type is classification. Let's merge this for now and address it in a separate PR.

Copy link
Collaborator

@ashwinvaidya17 ashwinvaidya17 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the efforts! This is a great addition

@github-actions github-actions bot added the Tests label Mar 2, 2023
@alexriedel1
Copy link
Contributor Author

alexriedel1 commented Mar 2, 2023

Sorry for the trouble, I wasn't able to run all the tests on my windows machine..

The datamodule now passes all the test in tests\pre_merge\datasets\test_synthetic_data.py and tests\pre_merge\datasets\test_datamodule.py:
tests\pre_merge\datasets\test_synthetic_data.py ..... [100%]
tests\pre_merge\datasets\test_datamodule.py ........... [100%]

that previously failed:

FAILED tests/pre_merge/datasets/test_datamodule.py::TestSubsetSplitting::test_abnormal_dir_omitted_from_dir
FAILED tests/pre_merge/datasets/test_datamodule.py::TestSubsetSplitting::test_abnormal_dir_omitted_synthetic
FAILED tests/pre_merge/datasets/test_synthetic_data.py::test_create_synthetic_dataset
FAILED tests/pre_merge/datasets/test_synthetic_data.py::test_create_from_dataset

I really wanted to remove the iteration over the dataframe in the folder dataset:

if mask_dir is not None:
mask_dir = _check_and_convert_path(mask_dir)
for index, row in samples.iterrows():
if row.label_index == 1:
rel_image_path = row.image_path.relative_to(abnormal_dir)
samples.loc[index, "mask_path"] = str(mask_dir / rel_image_path)

@codecov
Copy link

codecov bot commented Mar 2, 2023

Codecov Report

Patch coverage: 41.73% and project coverage change: -1.77 ⚠️

Comparison is base (1407cfc) 86.77% compared to head (f82993f) 85.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #942      +/-   ##
==========================================
- Coverage   86.77%   85.00%   -1.77%     
==========================================
  Files         165      169       +4     
  Lines        6319     6521     +202     
==========================================
+ Hits         5483     5543      +60     
- Misses        836      978     +142     
Impacted Files Coverage Δ
anomalib/data/base/dataset.py 97.72% <ø> (ø)
anomalib/post_processing/visualizer.py 71.83% <0.00%> (-1.03%) ⬇️
anomalib/data/folder_3d.py 16.47% <16.47%> (ø)
anomalib/data/base/depth.py 31.57% <31.57%> (ø)
anomalib/data/__init__.py 60.97% <33.33%> (-4.74%) ⬇️
anomalib/data/mvtec_3d.py 34.54% <34.54%> (ø)
anomalib/data/utils/image.py 51.51% <40.00%> (-0.95%) ⬇️
anomalib/data/utils/path.py 80.76% <80.76%> (ø)
...ib/models/components/feature_extractors/torchfx.py 92.98% <94.73%> (+0.38%) ⬆️
anomalib/data/base/__init__.py 100.00% <100.00%> (ø)
... and 4 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@samet-akcay
Copy link
Contributor

Thanks @alexriedel1! Much appreciated!

@samet-akcay samet-akcay merged commit 1bca117 into openvinotoolkit:main Mar 2, 2023
@alexriedel1 alexriedel1 deleted the mvtec_3d branch March 3, 2023 09:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Data Post-Processing The components that are related to post-processing Tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants