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

AugMix #607

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

AugMix #607

wants to merge 8 commits into from

Conversation

akarsakov
Copy link
Contributor

Refer to #488

Ported AugMix augmentation from: https://arxiv.org/abs/1912.02781

I've reproduced experiment on cifar-10 from article and result is really close to presented numbers.
Here is notebook with my experiment: https://colab.research.google.com/drive/1i2d9Ca7SVDuKp28E5GUhHNnutRoHh8hq

@akarsakov
Copy link
Contributor Author

DeepSource failed not in my changes... Probably it would be better to fix issues in separate PR.

@akarsakov akarsakov mentioned this pull request Apr 29, 2020
@MichaelMonashev
Copy link
Contributor

Если я верно понимаю идею albumentations, то ту есть удобное разделение на функции и трансформации.

Функции меняют изображение по ФИКСИРОВАННЫМ параметрам, которые им передают при вызове. Все функции находятся в https://github.com/albumentations-team/albumentations/blob/master/albumentations/augmentations/functional.py.

Трансформации сначала генерят СЛУЧАЙНЫЕ параметры в тех пределах, которые получили при вызове, а потом вызывают с этими параметрами функции из functional.py. Трансформации находятся в https://github.com/albumentations-team/albumentations/blob/master/albumentations/augmentations/transforms.py .

Такое разделение позволяет использовать использовать полезные функции независимо от трансформаций.

Поэтому содержимое aug_mix() нужно разделить на две части: на функцию и трансформацию. Всё, что генерит случайные параметры перенести в класс AugMix() и из него уже вызывать aug_mix(), передавая параметры, которые однозначно определят изменение изображения.

@MichaelMonashev
Copy link
Contributor

Ты реализовал полезные функции shear() и autocontrast(). Было бы здорово паровозиком ещё и сделать отдельные аугментации Shear() и Autocontrast().

This was referenced May 1, 2020
@akarsakov
Copy link
Contributor Author

@MichaelMonashev Привет!

Я сделал такой дизайн, потому что кроме параметров аугментаций (angle, posterize_bits и пр) есть еще внутренняя рандомизация вроде весов для смешивания и последовательность аугментаций. К тому же не очень удобно оперировать функцией с 10+ параметрами. В целом я вдохновлялся тем как сделан ElasticTransform. Готов вернуть все назад, просто навряд ли данная функция будет использоваться как "отдельный кирпичик" а рандомизация внутри все равно останется.

По поводу Shear и Autocontrast отличная мысль! Сделал issues для этого: #611 #612

@akarsakov
Copy link
Contributor Author

Еще я тут подумал, что наверно полезнее было сделать по типу Compose:

A.AugMix(
        [
            A.Autocontrast(),
            A.Equalize(mode='pil'),
            ...
        ],
        alpha=1,
        width=3,
        depth=3,
        mean=(..),
        std=(..)
    )

Либо более радикально, добавить композиции WeightedSum, Mix, Repeat:

    A.WeightedSum([
        A.Compose([
            A.Mix([
                A.Repeat([
                        A.OneOf([
                            A.Autocontrast(),
                            A.Equalize(),
                        ]),
                    ], num_repeats=(1, 3)),
                    A.Normalize(always_apply=True)
                    ], weights='dirichlet', alpha=1, width=3),
                A.Normalize(always_apply=True)
            ]),
            A.NoOp()
        ],
        mode='beta', 
        alpha=1
    )

Но наверно второй вариант это уже оверкил...

@MichaelMonashev
Copy link
Contributor

MichaelMonashev commented May 1, 2020

Я как раз думал ровно о том же и вот. Вот мои мысли.

У Compose() надо добавить параметр shufle=False|True . По дефолту shufle=False. Если он shufle=True, то все трансформации перемешиваются в случайном порядке. (Это видимо аналог твоего Mix()-а ).

Добавить аналог Compose(): Blend(n=(3,4)) (варианты названий Map, MapReduce, Mix, SplitJoin). Ещё вариант: вместо функции добавить параметр n у Compose(). Оно дублирует вход на n потоков, выполняет для каждого трансформации, а потом объединяет со случайными весами всё в одно изображение. Единственное, что мне тут не нравится: в одном месте делается 2 функции: дублирование на несколько потоков и обратное их слияние.

Для Bbox-ов, кстати, можно non maximum suppression применять при слиянии...

@MichaelMonashev
Copy link
Contributor

@BloodAxe , @Dipet , @ternaus , @albu , @creafz , @ZFTurbo , @qubvel . Хорошо бы всё это обсудить и хорошенько продумать...

@akarsakov
Copy link
Contributor Author

I've added Autocontrast and RandomShear transforms and made AugMix to accept list of transforms.
I didn't introduce new composition transforms in order not to overcomplicate the implementation for now.

P.S. deep source is still failing. Is it possible to run it on all codebase and fix all possible issues?

@MichaelMonashev
Copy link
Contributor

P.S. deep source is still failing. Is it possible to run it on all codebase and fix all possible issues?

этот дипсорс каждый день добавляет новые бредовые проверки. Это их фича такая. Поэтому я его фиксил его прибабахи отдельным коммитом внутри пул-реквеста. Это конечно неправильно, но зато помогает пройти проверки.

@MichaelMonashev
Copy link
Contributor

Ты написал много полезного кода и было бы жалко его терять весь из-за того, что его часть не нравиться ревьерам.

ИМХО, написанному будет проще попасть в albumentations, хотя бы частично, если его побить на отдельные PR-ы. Отдельно Autocontrast, отдельно RandomShear, отдельно bbox_shift_scale_rotate(), отдельно AugMix().

@minhlong94
Copy link

+1 for this feature.

Copy link
Contributor

@ORippler ORippler left a comment

Choose a reason for hiding this comment

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

My biggest issues I have so far with the implementation is:

  1. If there are additional_targets specified, your AugMix will lead to equal ops for each image, but each op will be parametrized differently across targets.
    Suggestion here: Sample the ops as well as their params in get_params, and provide both to augment_and_mix function

  2. You perform explicit ImageNet normalization, but people may want to normalize differently/not normalize at all. Also, if images have already been normalized, they will be normalized yet again by your implementation.
    Suggestion: Divert from the official reference implementation and leave normalization etc. to the user

)

def get_params(self):
return {"depth": random.randint(self.depth[0], self.depth[1]), "random_state": random.randint(0, 10000)}
Copy link
Contributor

Choose a reason for hiding this comment

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

Assigning a single int to depth here makes all augmentations of width have the same depth (this is in divergence to the reference implementation


if self.transforms is None:
self.transforms = [
Autocontrast(),
Copy link
Contributor

Choose a reason for hiding this comment

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

All transforms need to have p=1, such that they are always executed when selected by Augmix.

Maybe add a comment and change default values ?

@Dipet Dipet added Need more info Branch conflict Improvements needed Something not implemented well. The maintainers requested improvements. and removed Need more info labels Jun 11, 2022
@thiagoribeirodamotta
Copy link

Hello,
Are there any plans to merge this PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Branch conflict Improvements needed Something not implemented well. The maintainers requested improvements.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants