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

Allow to set a custom mask interpolation method #945

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

creafz
Copy link
Contributor

@creafz creafz commented Jul 4, 2021

This is POC to fix an issue in #850. It supports two styles for setting mask interpolation.

Option 1. Set a global value for mask interpolation through Compose:

t = A.Compose([
    A.Resize(128, 128),
], mask_interpolation=cv2.INTER_NEAREST)

Option 2. Override a value for mask interpolation for a single transform:

t = A.Compose([
    A.Resize(128, 128).set_mask_interpolation(cv2.INTER_NEAREST_EXACT),
])

@creafz creafz requested review from BloodAxe, ternaus and Dipet July 4, 2021 15:34
@creafz creafz marked this pull request as draft July 4, 2021 15:35
@creafz creafz changed the title [WIP] Allow to set a custom mask interpolation method Allow to set a custom mask interpolation method Jul 4, 2021
@creafz
Copy link
Contributor Author

creafz commented Jul 5, 2021

Or as an alternative for Option 2, maybe it is better to an explicit argument for all DualTransform subclasses, such as

t = A.Compose([
    A.Resize(128, 128, mask_interpolation=cv2.INTER_NEAREST_EXACT)
])

What do you think?

Comment on lines +121 to +135
if mask_interpolation not in {
cv2.INTER_NEAREST,
cv2.INTER_NEAREST_EXACT,
cv2.INTER_LINEAR,
cv2.INTER_LINEAR_EXACT,
cv2.INTER_AREA,
cv2.INTER_CUBIC,
cv2.INTER_LANCZOS4,
cv2.INTER_MAX,
}:
raise ValueError(
f"Value {mask_interpolation} is not supported. "
f"Choose one of the following methods: cv2.INTER_NEAREST, cv2.INTER_NEAREST_EXACT, cv2.INTER_LINEAR, "
f"cv2.INTER_LINEAR_EXACT, cv2.INTER_AREA, cv2.INTER_CUBIC, cv2.INTER_LANCZOS4, cv2.INTER_MAX"
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe better to create an Enum as class field for this flag?
With Enum error message would be much more clearly.

Comment on lines +136 to +138
for t in self.transforms:
if isinstance(t, BaseCompose) or (isinstance(t, DualTransform) and t.mask_interpolation is None):
t.set_mask_interpolation(mask_interpolation)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Strange condition, I am not sure that it will be work correctly, because mask_interpolation has no default value.

@Dipet
Copy link
Collaborator

Dipet commented Jul 5, 2021

Or as an alternative for Option 2, maybe it is better to an explicit argument for all DualTransform subclasses, such as

t = A.Compose([
    A.Resize(128, 128, mask_interpolation=cv2.INTER_NEAREST_EXACT)
])

What do you think?

I think approach better, but it is need to change __init__ method for all transforms. If we will do this, we must to add **kwargs as an argument for all transforms to have possibility to add new features simpler

@Dipet Dipet added the WIP label Jun 11, 2022
@LucaBonfiglioli
Copy link

please

@ternaus
Copy link
Collaborator

ternaus commented Mar 28, 2024

@LucaBonfiglioli Will look into this.

@zakajd
Copy link
Contributor

zakajd commented Jun 14, 2024

Heavy +1 for this PR. It's been 3 years since discussion and PR looks almost done. What's needed to finalise it? @ternaus @Dipet

The problems with resizing are very difficult to debug and lead to worse model performance. I had bad experience using Albumentations for segmentations and super resolutions tasks due to that and avoid using most of "geometrical" transformations for the same reason.

@ternaus
Copy link
Collaborator

ternaus commented Jun 14, 2024

@zakajd

We can do it.

As I understand, you are "Power User", meaning use the library for your task + use advanced features.

Sent you invitation to connect on LInkedIn, let's have video chat, your feedback / requests will help with the prioritization.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants