Reduce number of iterations in L0 tests #3173
Conversation
| @@ -64,41 +64,6 @@ def define_graph(self): | |||
| images = self.cmn(images, mirror=rng) | |||
| return images | |||
|
|
|||
| def check_cmn_cpu_vs_gpu(batch_size, dtype, output_layout, mirror_probability, mean, std, scale, shift, pad_output): | |||
There was a problem hiding this comment.
this test is redundant. Same thing is tested vs a python implementation. There's no need to test cpu vs gpu here.
jantonguirao
force-pushed
the
l0_reduce_iters
branch
from
f4fd47d
to
70d43db
Compare
|
Is the difference that significant to change from 5 to 3 iterations? In the former case we are sure to clear the output buffer queue at least twice. It has some appeal to do that IMO. |
| @@ -166,8 +166,7 @@ def check_multichannel_synth_data_vs_numpy(tested_operator, device, batch_size, | |||
| eii2 = RandomDataIterator(batch_size, shape=shape) | |||
| compare_pipelines(MultichannelSynthPipeline(device, batch_size, "HWC", iter(eii1), tested_operator=tested_operator), | |||
| MultichannelSynthPythonOpPipeline(get_numpy_func(tested_operator), batch_size, "HWC", iter(eii2)), | |||
| batch_size=batch_size, N_iterations=10, | |||
| eps = 0.2) | |||
| batch_size=batch_size, N_iterations=3, eps = 0.2) | |||
There was a problem hiding this comment.
Is it OK to reduce number of iterations for the test, which has significantly broader tolerance? Maybe we should ask the question, why this tolerance is broader?
There was a problem hiding this comment.
I believe it is OK. Some time ago we didn't think too much about the number of iterations. 10 is just an arbitrary number, I think. At some point, we decided that doing 3 iterations was enough (more than one iteration is necessary, since there are some issues when the operator doesn't clear the state properly between iterations). If we want to test more, we normally write a different test (different arguments, different kind of data, etc). Increasing the number of iterations is not equivalent of better coverage.
Regarding why the tolerance is broader, I don't have a good answer for it. This tests a whole pipeline with several operator chained, using randomly generated data, against a reference implemented in OpenCV. I assume the bigger eps is due to the data being pure noise, some subtle differences in the implementation might result in a larger error (interpolation, for instance). In line 236 we have the same test but with actual images, and the eps is much lower there.
| @@ -52,7 +52,7 @@ def test_color_twist_vs_old(): | |||
| rand_it2 = RandomDataIterator(batch_size, shape=(1024, 512, 3)) | |||
| compare_pipelines(ColorTwistPipeline(batch_size, seed, iter(rand_it1), kind="new"), | |||
| ColorTwistPipeline(batch_size, seed, iter(rand_it2), kind="old"), | |||
| batch_size=batch_size, N_iterations=16, eps=1) | |||
| batch_size=batch_size, N_iterations=3, eps=1) | |||
There was a problem hiding this comment.
I believe that 16 was chosen as an arbitrary number. Testing more iterations doesn't necessarily mean that we are doing better testing. If we wanted to increase test coverage, we should probably try with different arguments, for instance.
Signed-off-by: Joaquin Anton <janton@nvidia.com>
jantonguirao
force-pushed
the
l0_reduce_iters
branch
from
70d43db
to
b93f10e
Compare
|
!build |
|
CI MESSAGE: [2739796]: BUILD STARTED |
|
CI MESSAGE: [2739796]: BUILD PASSED |
JanuszL
added
the
important-fix
Signed-off-by: Joaquin Anton janton@nvidia.com
Description
What happened in this PR
Reducing number of iterations in L0 tests, and removing redundant cpu vs gpu test.
Additional information
L0 python tests
Are we OK with testing fewer iterations?
Checklist
Tests
Documentation
DALI team only
Requirements
REQ IDs: N/A
JIRA TASK: DALI-1001