-
Notifications
You must be signed in to change notification settings - Fork 222
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
[Enhancement] Add float datatype support to rocAucScore #3074
base: main
Are you sure you want to change the base?
Conversation
/intelci: run |
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.
Not backward-compatible:
miniconda3/envs/build/lib/python3.11/site-packages/daal4py/__init__.py:54: in <module>
from daal4py._daal4py import *
E ImportError: /.../miniconda3/envs/build/lib/python3.11/site-packages/daal4py/_daal4py.cpython-311-x86_64-linux-gnu.so: undefined symbol: _ZN4daal15data_management8internal11rocAucScoreERKNS_8services10interface19SharedPtrINS0_10interface112NumericTableEEES9_
This PR will be stopped until CI can track ABI changes, as this requirement is undocumented and not written in the oneDAL development rules, nor the checklist for PRs. |
@icfaust I haven't checked the code in detail, but doesn't the fastest way of calculating this metric involve computations of numbers up to "nrows^2" in a loop where they are added/subtracted to similar numbers? The fp32 data type only has integer-level precision up to 2^23 which is easy to exceed, after which the precision of the calculation might degrade. |
Description
The algorithm rocAucScore is currently forced to do all computation in doubles. This does not follow the coding standards of the codebase where it should template both the cpu ISA as well as the datatype (float or double). In order to properly interface to this code outside of DAAL (i.e. oneDAL), it must first maintain a float version as well. While introducing a datatype template, a default value of double is set in order to guarantee compatibility to daal4py. These changes do not have any impact on the performance or operation of daal4py or any code which currently interfaces rocAucScore, therefore performance benchmarking is unnecessary.
This PR is a precursor to enabling a oneDAL algorithm of rocAucScore. This completes issue #2740
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
Testing
Performance