Add tests for ResourceUtilizationCalculator #1341
Conversation
irenaby
force-pushed
the
ru_refactor_test
branch
from
67343aa to
1a3e384
Compare
irenaby
force-pushed
the
ru_refactor_test
branch
2 times, most recently
from
6adeb9b to
575dfd9
Compare
| @@ -0,0 +1,943 @@ | |||
| # Copyright 2024 Sony Semiconductor Israel, Inc. All rights reserved. | |||
…ttests automatically for coverage
irenaby
force-pushed
the
ru_refactor_test
branch
from
1158bbe to
01fcff1
Compare
|
@reuvenperetz please look at the github workflows. If we still need to exclude anything from coverage, let's talk. |
| enable_weights_quantization=pos_attr[1]), | ||
| activation_quantization_method=QuantizationMethod.POWER_OF_TWO, | ||
| quantization_preserving=False, | ||
| supported_input_activation_n_bits=[2, 4, 8], |
There was a problem hiding this comment.
are these values relevant for testing or just a placeholder? because 2 and 4 are not relevant values for "real scenarios" so unless they are used to test some specific behavior, maybe change them to 8, 16 which are the relevant options for this field
There was a problem hiding this comment.
They are just a placeholder, but anyway the calculator is not and should not be aware of what real scenarios are. If anything I would change it to some random values, but it's not used so it doesn't really matter
| pass | ||
|
|
||
|
|
||
| def build_node(name='node', canonical_weights: dict=None, qcs=None, input_shape=(4, 5, 6), output_shape=(4, 5, 6), |
There was a problem hiding this comment.
It seems that the setup for this test (tpc configs, node configs, graph mock, etc.) is a very good setup for many tests that would require such objects.
Maybe we extract it to a 'util' tests section so it would also be available in other tests?
It's probably better to do it in a separate PR, but maybe we can create a kind of "utils API for tests", otherwise I see a case where all this code is duplicated across many tests without changing anything.
There was a problem hiding this comment.
My only concern is that as a util it will be constantly extended to be more and more configurable, and become a mess. Also someone can decide to change the functionality a bit thinking it doesn't matter, when some previous tests count on that. We should be careful what is really a common utility, and what is the minimal implementation for a specific test.
Pull Request Description:
Add unit tests for Resource Utilization Calculator.
Minor updates, mostly validations and errors.
Checklist before requesting a review: