-
Notifications
You must be signed in to change notification settings - Fork 221
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
Use parallel reduce threading primitive in covariance algorithm #3126
base: main
Are you sure you want to change the base?
Conversation
{ | ||
return; | ||
} | ||
tls_data_t<algorithmFPType, cpu> result = tbb::parallel_reduce( |
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.
Wouldn't it be better to make a NUMA-aware version of this function that would first reduce within nodes and then globally?
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.
@david-cortes-intel Thank you for looking into this.
I would prefer to go step-by-step with NUMA.
If a non-NUMA version is enough to improve performance, I would be more likely to add it first.
If not, than I'll probably add NUMA awareness into the primitives.
I also have to say that this version of the code is just an initial quick and dirty commit. The code will change a lot further. At least it should pass the testing and all the tbb::
stuff should be moved into threading layer.
…:tls_data_t struct
Pull changes from main branch
Description
Add a comprehensive description of proposed changes
List associated issue number(s) if exist(s): #6 (for example)
Documentation PR (if needed): #1340 (for example)
Benchmarks PR (if needed): IntelPython/scikit-learn_bench#155 (for example)
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