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

[pota-quantization-value-test] Set right mode #12634

Merged
merged 1 commit into from
Feb 15, 2024
Merged

Conversation

mhs4670go
Copy link
Contributor

This commit fixes the bug where the mode was not set properly.

Related: #12526
ONE-DCO-1.0-Signed-off-by: seongwoo [email protected]

This commit fixes the bug where the mode was not set properly.

ONE-DCO-1.0-Signed-off-by: seongwoo <[email protected]>
@mhs4670go mhs4670go added the PR/ready for review It is ready to review. Please review it. label Feb 15, 2024
@mhs4670go mhs4670go requested a review from jinevening February 15, 2024 02:32
@mhs4670go
Copy link
Contributor Author

@Seunghui98

pota-quantization-value-test test will be passed after this PR.

@Seunghui98
Copy link
Contributor

@mhs4670go
Q) Why is it related to setting mode and solving this bug? :)

@mhs4670go
Copy link
Contributor Author

@mhs4670go Q) Why is it related to setting mode and solving this bug? :)

One of the tests that failed is pota-quantization-value-test.

The following tests FAILED:
         64 - pota_quantization_test_with_config (Failed)
         76 - circle-interpreter-test (Failed)
Errors while running CTest

@mhs4670go
Copy link
Contributor Author

@Seunghui98 I've said the detail in the offline since it's a bit complicated to be explained by comments. But let me try as much as possible.

pota-quantization-value-test tests the values from outputs of each quantization steps. The test names each steps as "mode". And, I've written a script that does testing according to given mode.

modes_to_expected_folder = {
'fake_quantization': 'fake_quantization',
'mixed_fake_quantization': 'fake_quantization',
'record_minmax': 'record_minmax',
'parallel_record_minmax': 'record_minmax',
'quantization': 'quantization',
'mixed_quantization': 'quantization',
'weights_only_quantization': 'wo_quantization'
}
modes_to_input_h5_suffix = {
'fake_quantization': 'fake_quantized.circle.h5',
'mixed_fake_quantization': 'fake_quantized.mixed.circle.h5',
'record_minmax': 'minmax_recorded.circle.h5',
'parallel_record_minmax': 'parallel_minmax_recorded.circle.h5',
'quantization': 'quantized.circle.h5',
'mixed_quantization': 'quantized.mixed.circle.h5',
'weights_only_quantization': 'wo_quantized.circle.h5'
}

But, I mistakenly gave a wrong mode for some tests. Even though the test failed at first, the second run passed because the required output artifacts are generated from other test which is the one that I've mistakenly given.

For example, this is the right test order.

./test.py --mode mixed_record_minmax
./test.py --mode mixed_quantization # it depends on the artifacts from `mixed_record_minmax`

./test.py --mode record_minmax
./test.py --mode quantization # it depends on the artifacts from `record_minmax`

But, I mistakenly wrote tests like below.

./test.py --mode mixed_record_minmax
./test.py --mode quantization # WRONG!!

./test.py --mode record_minmax
./test.py --mode quantization

Above second line failed because the quantization mode test depends on the artifacts of record_minmax not mixed_record_minmax.

But, second run passed because record_minmax mode test has run at the first run so there are outputs in the test artifact directory.


So, I fixed the wrong mode.

I think this is because it is important to remove previously generated test artifacts before running the testing.

@Seunghui98
Copy link
Contributor

@mhs4670go

Thank you for letting me know. :)

Copy link
Contributor

@Seunghui98 Seunghui98 left a comment

Choose a reason for hiding this comment

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

LGTM :)

Copy link
Contributor

@jinevening jinevening left a comment

Choose a reason for hiding this comment

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

LGTM

@jinevening jinevening merged commit 6a1be91 into Samsung:master Feb 15, 2024
6 checks passed
@mhs4670go mhs4670go deleted the re2 branch April 29, 2024 12:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR/ready for review It is ready to review. Please review it.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants