-
Notifications
You must be signed in to change notification settings - Fork 132
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
SSSP adaptive #351
base: master
Are you sure you want to change the base?
SSSP adaptive #351
Conversation
a196c96
to
2455519
Compare
2455519
to
e0d235d
Compare
Local tests look good. No deadlocking happened. |
Weird. Okay, I'm doing a release build with gcc 10 and running the command line |
This is on one of the 40 core machines. |
My bad. I'm fiddling with it a bit more. It's NOT actually deadlocking. It's just running extremely slowly with high core counts, probably due to contention internally somewhere. |
Thus far: 20 cores runs slowly. I haven't actually seen it finish for any higher core count than that. 40 cores doesn't appear to work at all, but maybe I haven't given it enough time. |
Just confirmed. It DOES actually terminate even with 40 cores. It's just horribly slow, so we're not running up against a bug per se, just really pathological performance behavior. |
Interesting. Note that Adaptive OBIM uses per-thread locks and requires all locks when merging/unmerging whereas OBIM does not. We may optimize it to a per-socket merging/unmerging. Not sure if we should do this now for FMM or just use the old OBIM for that. |
Yah, given that the deadline is coming up, we may need to just use OBIM. For FMM, adaptive OBIM would be nice to have for completeness in the benchmarks, but it's not essential for any point we're trying to make. I was hoping it'd let us eliminate the problem-specific round off factor that has to be tuned for performance, but even if it were fully functional right now, adaptive OBIM doesn't quite get us that far. It doesn't handle floats gracefully so we still have to do a little rounding. |
The test failures from the louvain clustering code are unrelated. Do we want to merge this new SSSP version? I've been keeping it around for debugging but since it performs so poorly right now we may want to leave it as a pull request until we sort out what's going on. |
Let's leave it here. Thanks! |
dbbe96c
to
b698f8c
Compare
This adds yet another variant of SSSP. The important thing here is that it provides a concrete example that exercises the new Adaptive OBIM scheduler.