[CK_BUILDER] validation #3471
Open
[CK_BUILDER] validation #3471
Conversation
This file contains hidden or 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
This is the simplest most implementation for validation.
This implements very barebones Googletest integration by implementing a custom Matcher which calls ckt::validate().
This is a better version of the existing hip_check_error implementations. It has the following improvements: - Throws exceptions. Thos way the underlying unit testing implementation can cleanly catch & report errors, instead of a test just exiting in a hostile way. - Uses std::source_location for source locations, so that we don't have to use a macro for the root "checking" function. - Allows us to define and catch different types of errors without additional required control flow.
This implementation just performs the comparison as if it was a flat piece of memory, meaning including stride data. While this is primarily done because its much simpler, checking strides is also useful so that we can check that there were no out-of-bounds writes. In the future, the implementation of compare_tensors may need to change such that the "actual" and "expected" tensors can have a different stride/layout.
The specialization of DataTypeToCK was missing for DataType::U8. This commit also changes the test such that if there is a variant missing in the future, we will get a warning/compile error.
CKB testing should include user-readable error messages if a tensor fails to validate. For this purpose, ckt::validate now returns a "ValidationReport" a set of "cases" which hold information about how and why a particular check failed. The testing infrastructure (see testing_utils.hpp) can then turn this metadata into readable error messages. This mechanism is to be expanded.
This adds initial unit tests for CKB testing's validation utilities. This mainly adds tests for the current version of ValidationReport, as well as some utilities for performing the tests (which are also tested themselves).
This new version compares the tensors actually, rather than comparing just the backing buffers. This should make it easier to gather coordinates, as well as allow us to compare non-packed kernels.
This nets us a few advantages: - More static validation of the rank (we don't need to explicitly check). - Easier metaprogramming in validation kernels. There are also some disadvantages though: - More metaprogramming in general. - Harder to use everywhere else.
Putting this code into a separate header allows us to reuse it for data generation and more validation stuff.
Now that we have this sort of abstract implementation, we can re-use this implementation.
This cleans up the test a bit.
This frees up the Extent name for use with the TensorDescriptor. Since
it will be used to create extents like ckt::Extent{1, 2, 3}, it makes
sense to use this sorter name for that. The FilterExtent typename will
mostly be used in contexts where it can be inferred, so its not so its
better to use a longer name there.
This functionality is useful outside of unit_validation.cpp, so move it to a more common location.
…tor.hpp At this point the tensor descriptor stuff has become larger than the actual tensor buffer stuff, so lets move it into a separate file.
Snektron
requested review from
aosewski,
carlushuang,
geyyer,
poyenc and
qianfengz
as code owners
Snektron
requested review from
ThomasNing,
afagaj,
andriy-ca,
asleepzzz,
bartekxk,
cgmillette,
coderfeli,
shumway,
tenpercent and
vidyasagar-amd
as code owners
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.
Proposed changes
This pull request builds on #3267 by proving the "validation" infrastructure, the means to compare a set of
Outputs.Design
The design of the validation infrastructure is relatively straight forward:
validate()implementation, which should be implemented in a similar way that the other functions/types fromtesting.hppare implemented.validate()returns aValidationReport, which is a structure that keeps all relevant information about comparing the tensors from twoOutputs. Note that crucially,validate()should not do any reporting by itself. Rather, glue logic should be implemented by the user to turnValidationReportinto a relevant error message.testing_utils.hpp, itsMatchesReference(). This functionality is relatively barebones right now, it will be expanded upon in a different PR to keep the scope of this one down.Implementation
The comparison is done on the GPU (using an atomic for now), to keep tests relatively quick. Some notable items from this PR:
tensor_foreachwhich invokes a callback on every element of a tensor.TensorDescriptorhas a rank which is known at compile-time, so I've changed the implementation ofTensorDescriptorfor that. I felt like it was a better approach than keeping it dynamic, for multiple reasons:tensor_foreachand other comparison code.