-
Notifications
You must be signed in to change notification settings - Fork 179
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
[CI, enhancement] Add ability to build with gcov
, adding C++ code coverage for the onedal
folder in codecov
#2249
[CI, enhancement] Add ability to build with gcov
, adding C++ code coverage for the onedal
folder in codecov
#2249
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Flags with carried forward coverage won't be shown. Click here to find out more. |
onedal
folder
onedal
folderonedal
folder in codecov
onedal
folder in codecovgcov
, adding C++ code coverage for the onedal
folder in codecov
/intelci: run |
@icfaust What would be the right way to generate a report locally? I haven't been able to get an interactive html report from this PR. |
You should be able to use |
Meaning: it's not generated as part of the |
@icfaust Can you attach here an .html report of the C++ code coverage that would be generated? |
I recommend looking at codecov: #2249 (comment) It has a file explorer and unifies all the builds together: https://app.codecov.io/gh/uxlfoundation/scikit-learn-intelex/pull/2249/tree |
Co-authored-by: david-cortes-intel <david.cortes@intel.com>
Description
Coverage statistics have been recently added to sklearnex via codecov. This was focused on the Python code, but C++ is the other important aspect for integrating oneDAL (representing a fifth of the code in the repo). This PR does the following:
It does this by using the
gcovr
package, and by a bash script (generate_coverage_files.sh
). Note, to use gcov, the build Numpy must be installed, otherwise it will fail (usually changed in the requirements-test.txt installation). Therefore, the bash script uses aNUMPY_BUILD
environment variable in order to store this information. This does not impact the daal4py build, as it is considered legacy code.Consequences:
Note: windows visual studio does not easily maintain a code coverage tool, and is therefore neglected. The majority of code is operated with the DPC build, with only a small segment being missed.
No performance benchmarks necessary.
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