Fix VideoReader CPU only variant #3660
Conversation
Signed-off-by: Albert Wolant <awolant@nvidia.com>
|
!build |
|
I would update test_dali_cpu_only.py as |
|
CI MESSAGE: [3907820]: BUILD STARTED |
Not sure what you mean here? There are no python tests for this at this time. |
Sorry, I was not precise. Here you test the decoder, but I wanted to make sure that the |
|
Operator is tested with C++ tests as well and I adjusted them to be picked up by the cpu only test job as well in this PR. |
I think we should have at least a basic test inside test_dali_cpu_only.py, otherwise it will be hard to track where each operator is tested. |
| @@ -21,11 +21,11 @@ | |||
|
|
|||
|
|
|||
| namespace dali { | |||
| class FramesDecoderTest : public VideoTestBase { | |||
| class FramesDecoderTest_CpuOnlyTests : public VideoTestBase { | |||
There was a problem hiding this comment.
I think gtest is against putting _ in test names.
Also, why is this name change necessary?
There was a problem hiding this comment.
We use this sufix to pick up tests we want to run in the CPU only test. This is already existing mechanism, so I just used it.
Possible change in that to comply to gtest recommendation is out of scope for this, I think.
There was a problem hiding this comment.
Here is the test run: https://github.com/NVIDIA/DALI/blob/main/qa/TL0_cpu_only/test.sh#L34
There was a problem hiding this comment.
And here is how we use it in other tests: https://github.com/NVIDIA/DALI/blob/main/dali/kernels/slice/slice_cpu_test.cc#L75
There was a problem hiding this comment.
I think the main idea of the slice test here was to check CPU implementation than to check if it can run without GPU. We have only one such native test with that intention - CApiTest.
There was a problem hiding this comment.
The pattern that we use will work regardless of _.
|
CI MESSAGE: [3907820]: BUILD FAILED |
Signed-off-by: Albert Wolant <awolant@nvidia.com>
| @@ -603,6 +604,9 @@ def create_wav_files(): | |||
| check_no_input(fn.readers.nemo_asr, manifest_filepaths=[nemo_asr_manifest], dtype=types.INT16, downmix=False, | |||
| read_sample_rate=True, read_text=True, seed=fixed_seed) | |||
|
|
|||
| def test_video_reader(): | |||
Signed-off-by: Albert Wolant <awolant@nvidia.com>
|
!build |
|
CI MESSAGE: [3908645]: BUILD STARTED |
|
CI MESSAGE: [3908645]: BUILD PASSED |
…into awolant/frames_decoder_gpu
|
!build |
|
CI MESSAGE: [3911220]: BUILD STARTED |
|
CI MESSAGE: [3911220]: BUILD PASSED |
| EXPECT_TRUE(strstr( | ||
| e.what(), | ||
| make_string("Failed to open video file at path ", path).c_str())); |
There was a problem hiding this comment.
How about
EXPECT_THROW(FramesDecoder(path), DALIException)
?
I don't think you can match the error message, so up to you
There was a problem hiding this comment.
Yeah, I wanted to make sure that the message is the one I want and it reflects what happen, so I'm going to leave it for now.
If this keeps reoccurring we can think on some solution that would do both.
* Fix CPU video tests Signed-off-by: Albert Wolant <awolant@nvidia.com>
* Fix CPU video tests Signed-off-by: Albert Wolant <awolant@nvidia.com>
* Fix CPU video tests Signed-off-by: Albert Wolant <awolant@nvidia.com>
Signed-off-by: Albert Wolant awolant@nvidia.com
Category:
Bug fix
Description:
VideoReaderCpuwas unnecessarily dependent on the GPU. Removed the dependency and added proper tests for CPU only test suite.Additional information:
Affected modules and functionalities:
Key points relevant for the review:
Checklist
Tests
Documentation
DALI team only
Requirements
REQ IDs: N/A
JIRA TASK: DALI-2588