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

SSSP adaptive #351

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

insertinterestingnamehere
Copy link
Member

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.

@bozhiyou
Copy link
Member

Local tests look good. No deadlocking happened.

@insertinterestingnamehere
Copy link
Member Author

insertinterestingnamehere commented Jul 28, 2020

Weird. Okay, I'm doing a release build with gcc 10 and running the command line ./sssp-cpu "/net/ohm/export/iss/inputs/scalefree/rmat10.gr" -delta 1 -algo=deltaStepAdaptive -t=40.

@insertinterestingnamehere
Copy link
Member Author

This is on one of the 40 core machines.

@insertinterestingnamehere
Copy link
Member Author

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.

@insertinterestingnamehere
Copy link
Member Author

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.

@insertinterestingnamehere
Copy link
Member Author

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.

@bozhiyou
Copy link
Member

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.

@insertinterestingnamehere
Copy link
Member Author

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.

@insertinterestingnamehere
Copy link
Member Author

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.

@bozhiyou
Copy link
Member

Let's leave it here. Thanks!

@bozhiyou bozhiyou force-pushed the master branch 2 times, most recently from dbbe96c to b698f8c Compare August 28, 2023 22:38
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