-
Notifications
You must be signed in to change notification settings - Fork 443
Process FP16 output and output to Console and CSV file #176
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
Conversation
…r tensor comparisons
| std::getline(actualFileStream, actualValue, '\n'); | ||
| float actualValueNum = 0; | ||
| float expectedValueNum = 0; | ||
| try |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check if empty
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
| } | ||
| //If the string values aren't floats then ignore | ||
| } | ||
| if (std::abs(actualValueNum - expectedValueNum) > epsilon * std::abs(expectedValueNum) + epsilon) //Check if the values are too different. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
relative tolerance is the typical term.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
Tools/WinMLRunner/src/OutputHelper.h
Outdated
|
|
||
| template<typename T> | ||
| void WriteTensorResultToCSV(T* m_Res, uint32_t iterationNum, const CommandLineArgs &args, uint32_t &capacity, std::string &featureName) | ||
| void ProcessTensorResult(com_ptr<ITensorNative> itn, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Try to detemplatize so there isn't code repetition. You can just use a switch statement in the innermost loop.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed out most of the code that's unnecessary for being inside the template. Case switch statement for tensorkind will have to exist somewhere. I chose to have it in the helper method that calls ProcessTensorResult and leave ProcessTensorResult as template for using typename T for looping through the buffer.
fdwr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks fine. Add some desired changes, but nothing blocking. Thanks for unblocking Intel and Indy for float16.
This change primarily addresses these main points:
Before this change, we access the output tensor twice and it was inefficient and had code bloat. It accessed it once to dump the output tensor to CSV and another time to print the max value to console. With this change, there is only one method to access the output tensor once. PrintOrSaveEvaluationResults is a helper method which controls the cases for the output tensorkind and it calls ProcessTensorResult which is a templated method that handles the access of the output tensor depending on the data type.
This change fixes the CompareTensor helper method inside the tests so that the comparing or the tensor values is more fair. Epsilon is used here to determine the threshold where the expected tensor value and actual tensor value are too different.
This change adds squeezenet fp16 model. It was converted with WinMLTools 1.3.0.
Output Tensor for FP16 tests and test data were added as well.