Skip to content
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

perf: Combinable histograms for angle module parallel execution #1937

Draft
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

RobBuchananCompPhys
Copy link
Contributor

@RobBuchananCompPhys RobBuchananCompPhys commented Jun 11, 2024

I've worked on parallelising the module using a similar approach to the previous multithreading work on #1882 (SiteRDF).
Running into a snag where Angle tests are failing only when execution_policy is par (but passing when running sequentially!).
I'm guessing this is due to a part of the histogram binning process being non-thread safe, and I believe I have narrowed it down to the Histogram3D, since the output from the 1D and 2d histograms looks correct in gdb. Surprising, as the histogram classes seem to all effectively be wrappers to a linear array, so I am still trying to figure out where this is going wrong in parallel 3D binning.

@trisyoungs
Copy link
Member

Regarding this the updated module runs really slowly in parallel, so there's something bad happening somewhere. I have taken a bit of a diversion and decided to handle #1927 and then add one for your module so we can do a before/after comparison too. This is something we should commit do doing for every module we convert in this way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants