port AA tests #7927

pmeier · 2023-09-01T10:19:26Z

Some questions that are up for discussion:

Since we have changed the random sampling in v2, we cannot easily compare the AA transforms to their v1 equivalent. It is doable, but leads to complicated monkeypatching / mocking, e.g.

vision/test/test_transforms_v2_consistency.py

Lines 729 to 743 in fb115c2

    
           le = len(t._AUGMENTATION_SPACE) 
        
           keys = list(t._AUGMENTATION_SPACE.keys()) 
        
           randint_values = [] 
        
           for i in range(le): 
        
               # Stable API, op_index random call 
        
               randint_values.append(i) 
        
               # Stable API, if signed there is another random call 
        
               if t._AUGMENTATION_SPACE[keys[i]][1]: 
        
                   randint_values.append(0) 
        
               # New API, _get_random_item 
        
               randint_values.append(i) 
        
           randint_values = iter(randint_values) 
        
           mocker.patch("torch.randint", side_effect=lambda *arg, **kwargs: torch.tensor(next(randint_values))) 
        
           mocker.patch("torch.rand", return_value=1.0)

IMO, it was a good thing that we implemented this thorough check when we were actively working on implementing the AA transforms for v2 to avoid accidental breakages. However, now that they are ported and checked, we can probably drop them. We do not guarantee BC for randomly sampled parameters and thus, these test offer little value. Worse they might break in the future if we change something about the random sampling.

The checks for the v1 transforms call the transform multiple times:

vision/test/test_transforms.py

Lines 1570 to 1571 in fb115c2

for _ in range(100):

img = transform(img)

vision/test/test_transforms_tensor.py

Lines 590 to 592 in fb115c2

    
           for _ in range(25): 
        
               _test_transform_vs_scripted(transform, s_transform, tensor) 
        
               _test_transform_vs_scripted_on_batch(transform, s_transform, batch_tensors)

I presume this is to get a better coverage of the randomly sampled ops. However, this also leads to quite long test times, e.g.

❯ time pytest test/test_transforms.py::test_augmix &> /dev/null
pytest test/test_transforms.py::test_augmix &> /dev/null  1143,96s user 71,33s system 1156% cpu 1:45,11 total

Since we have a frozen RNG seed, different parametrization will not increase coverage of the sampled ops. One option could be to reproducibly set the seed based on the parametrization, i.e. seed = hash(("interpolation", InterpolationMode.NEAREST)). Since we run multiple parametrizations for each transform, that should give us good coverage without the need to run the transform multiple times for each parametrization.

Thoughts?

cc @vfdev-5

pytorch-bot · 2023-09-01T10:19:29Z

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/vision/7927

📄 Preview Python docs built from this PR

Note: Links to docs will display an error until the docs builds have been completed.

❗ 1 Merge Blocking SEVs

There is 1 active merge blocking SEVs. Please view them below:

(merge blocking) Networking issue preventing multiple CI jobs to access GH API

If you must merge, use @pytorchbot merge -f.

❌ 1 New Failure

As of commit a4de6f0 with merge base d0e16b7 ():

NEW FAILURE - The following job has failed:

unittests-macos (3.8, macos-m1-12) / macos-job (gh)

This comment was automatically generated by Dr. CI and updates every 15 minutes.