-
Notifications
You must be signed in to change notification settings - Fork 66
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
Matrix row size guess is set at 8; may be too small for many use cases #74
Comments
The idea we discussed is that the code already loops through the whole mesh, so it could just count the number of entries in each row of the graph and have an exact number per row. The first-pass improvement would be to use CrsGraph's StaticProfile constructor with an array of the number of entries in each row. However, see trilinos/Trilinos#119 . The better improvement would be to construct the local graph as a Kokkos::StaticCrsGraph (via a second pass through the mesh), then hand that off to Tpetra::CrsGraph's constructor. This would be the best approach moving forward. |
Agreed. We plan on pursuing the above approach in #75. Depending on the time frame, it may be better to simply do that and not worry about this ticket. Volunteers? |
I'll be happy to help guide the process. I just still have a lot to learn about how one iterates over meshes in these codes, so I might not be as effective actually doing the typing just yet ;-) The "CrsGraph with StaticProfile" approach (count first, create CrsGraph with StaticProfile and the exact count for each row, then construct the graph as you currently do) would likely give immediate speedup for graph construction in the MPI-only case. You could first make sure that works, then build on that to do Kokkos-based thread-parallel graph construction. Doing this in two steps would give you a more fair performance comparison for the implementation of thread-parallel graph construction. |
Mark, I have Christian's prototype of kokkos-based graph construction from last year, which I've updated so it passes current tests. Perhaps you and I can get together and talk about the details of putting in the static-profile approach and the kokkos-based construction. We could look through the code and see how Christian's changes would interact (if at all) with the static-profile changes, and decide what order to do things. I like the idea of doing it in two steps for a good comparison. And I can tell/show you how the mesh iteration works. |
I just did a little more perusing of the graph-building approach in nalu's TpetraLinearSystem.C. It's not quite as trivial as we thought to replace the '8' with a list of row-lengths. I'll put together a ppt slide or two describing the sequence of operations involved in our graph-construction. I don't think the change will be too bad, but may require a little more code rearranging than we first thought. |
Thanks @alanw0 for the slides! I'll take a look. |
@alanw0, yes:) I looked at the code today and came to the same conclusion on the static graph. |
@sayerhs and @michaelasprague, please make sure that this ticket was captured in Jira. It is related to ECP FY17Q4 |
Initial guess at row size is set at 8 even though we can support CVFEM 3x3 momentum:
globallyOwnedGraph_ = Teuchos::rcp(new LinSys::Graph(globallyOwnedRowsMap_, ownedPlusGloballyOwnedRowsMap_, 8));
We should set that final number based on the local and shared row map to get the size of the stencil 100% correct.
The text was updated successfully, but these errors were encountered: