Add Tensor<T> helper class for tests #7830
Open
+901
−1,661
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![copybara-service[bot]](https://avatars.githubusercontent.com/in/44061?s=60&v=4){kind=small}
copybara-service
bot
force-pushed
the
test_727983393
branch
from
bebff24 to
0e7f96a
Compare
copybara-service
bot
changed the title
Tensor<T>, a helper class for multi-dimensional arrays that are frequently used in our tests (but currently are duplicating extent/stride computations frequently). Second is modifying two tests (slice and transpose) to make some changes:
copybara-service
bot
force-pushed
the
test_727983393
branch
8 times, most recently
from
d25344a to
5f8c80e
Compare
copybara-service bot
pushed a commit
that referenced
this pull request
Feb 19, 2025
…emory This causes crashes on windows, uncovered in #7830. PiperOrigin-RevId: 728501674
copybara-service bot
pushed a commit
that referenced
this pull request
Feb 19, 2025
…emory This causes crashes on windows, uncovered in #7830. This bug appears with `xnn_create_runtime_v4`, other paths that allocate workspaces have matching allocate/free calls. PiperOrigin-RevId: 728501674
copybara-service bot
pushed a commit
that referenced
this pull request
Feb 19, 2025
…emory This causes crashes on windows, uncovered in #7830. This bug appears with `xnn_create_runtime_v4`, other paths that allocate workspaces have matching allocate/free calls. PiperOrigin-RevId: 728501674
copybara-service bot
pushed a commit
that referenced
this pull request
Feb 19, 2025
…emory This causes crashes on windows, uncovered in #7830. This bug appears with `xnn_create_runtime_v4`, other paths that allocate workspaces have matching allocate/free calls. PiperOrigin-RevId: 728550543
copybara-service
bot
force-pushed
the
test_727983393
branch
3 times, most recently
from
e3acf6c to
417981a
Compare
This CL does two things: first is adding `Tensor<T>`, a helper class for multi-dimensional arrays that are frequently used in our tests (but currently are duplicating extent/stride computations and other buffer related logic frequently). Second is modifying a few tests to make some changes: - Currently, subgraph tests compare subgraph to operator results. This changes tests to directly check the output, without running the operator code. - Currently, subgraph tests run a single random variation, and getting good coverage requires running the test many times. This changes the subgraph tests to test cover many more permutations in a single run. - Currently, subgraph tests dig into the internal implementation details of subgraphs (e.g. checking xnn_node_value state). This makes sense in some cases (e.g. fusion tests), but it is both hard to be certain that this covers real usage, and is brittle. IMO, tests should (as much as possible) attempt to verify the behavior is as expected via the APIs that are visible to the user of the thing they are testing. For the subgraph API, that means we should just make sure the subgraph works as expected. - Currently subgraph tests are very verbose. IMO this is a problem because it discourages writing tests. This CL adds bfloat16 test coverage for constant_pad (and enables that operation, which previously didn't work for any good reason) with just 3 marginal lines of code, whereas before it would have added several hundred lines of code (copy/paste + modifications) This change required a few minor cleanups: - `xnnpack::Buffer<T>` needs to be able to distinguish between "extra bytes" and real data. - There is now some overlap between `RuntimeTester` and `SubgraphTester`. I think we should deprecate `RuntimeTester` and consolidate everything in `SubgraphTester`, because we can't return `RuntimeTester` from the base class `SubgraphTester` builder methods. This is a minor difficulty, but it also seems like the reason to separate them is minor too. PiperOrigin-RevId: 727983393
copybara-service
bot
force-pushed
the
test_727983393
branch
from
417981a to
51a0ed5
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add Tensor helper class for tests
This CL does two things: first is adding
Tensor<T>, a helper class for multi-dimensional arrays that are frequently used in our tests (but currently are duplicating extent/stride computations and other buffer related logic frequently). Second is modifying a few tests to make some changes:This change required a few minor cleanups:
xnnpack::Buffer<T>needs to be able to distinguish between "extra bytes" and real data.RuntimeTesterandSubgraphTester. I think we should deprecateRuntimeTesterand consolidate everything inSubgraphTester, because we can't returnRuntimeTesterfrom the base classSubgraphTesterbuilder methods. This is a minor difficulty, but it also seems like the reason to separate them is minor too.