oneDAL
oneDAL copied to clipboard
oneapi-src
FIX: Check each error code instead of just last
Description
This PR modifies the logic for summary statistics to make a check after each call to MKL to verify that no error occurred, instead of checking only the last one. Theoretically it shouldn't matter as only the 'compute' step is somewhat expected to fail when the rest of the procedure is correct, but internal errors in logic can still happen.
PR should start as a draft, then move to ready for review state after CI is passed and all applicable checkboxes are closed. This approach ensures that reviewers don't spend extra time asking for regular requirements.
You can remove a checkbox as not applicable only if it doesn't relate to this PR in any way. For example, PR with docs update doesn't require checkboxes for performance while PR with any change in actual code should have checkboxes and justify how this code change is expected to affect performance (or justification should be self-evident).
Checklist to comply with before moving PR from draft:
PR completeness and readability
- [x] I have reviewed my changes thoroughly before submitting this pull request.
- [x] Git commit message contains an appropriate signed-off-by string (see CONTRIBUTING.md for details).
- [x] I have added a respective label(s) to PR if I have a permission for that.
- [x] I have resolved any merge conflicts that might occur with the base branch.
Testing
- [x] I have run it locally and tested the changes extensively.
- [x] All CI jobs are green or I have provided justification why they aren't.
CI failures are either timeouts, or not related to the changes here.
@Vika-F In the current code and in the modified one that you posted, if passing method == __DAAL_VSL_SS_METHOD_FAST_USER_MEAN, wouldn't the means that it calculates either way get overridden by adding __DAAL_VSL_SS_ED_MEAN?
@Vika-F I've rewritten these files using objects with destructors now. Let me know if this is what you had in mind in your comment.
@david-cortes-intel Sorry, I've lost this one =
Yes, this is better than gotos in my opinion.
The only thing which bothers now is return 1 for memory allocation errors. Is it possible to use a better constant?
@david-cortes-intel Sorry, I've lost this one = Yes, this is better than gotos in my opinion.
The only thing which bothers now is
return 1for memory allocation errors. Is it possible to use a better constant?
Yes. But what should that constant be? Is there some table describing what codes should be used for which error types?
Yes. But what should that constant be? Is there some table describing what codes should be used for which error types?
Is it possible to use this one, casted to int: https://github.com/uxlfoundation/oneDAL/blob/main/cpp/daal/include/services/error_indexes.h#L150 ?
Yes. But what should that constant be? Is there some table describing what codes should be used for which error types?
Is it possible to use this one, casted to int: https://github.com/uxlfoundation/oneDAL/blob/main/cpp/daal/include/services/error_indexes.h#L150 ?
Modified to use that error code instead.
