trilinos / Trilinos

Primary repository for the Trilinos Project
https://trilinos.org/
Other
1.19k stars 565 forks source link

Tpetra: Reduce Compilation of ordinal-as-scalar objects #13094

Open csiefer2 opened 2 months ago

csiefer2 commented 2 months ago

In order to provide an analogue for Epetra_IntVector, Tpetra rebuilds sizable portions of its library with Scalar=GO and Scalar=int (if int is not the GO). We clearly need Vector for moving around things like MPI rank ids, but it isn't obvious we need as much aGO/int-as-Scalar as we have in the code.

We'd like to (optionally) remove a bunch of this extra compilation of GO/int-as-Scalar to reduce build time / library size (as requested by @ibaned), for applications that don't need it (like his).

This can be done by mucking with the ETI logic in packages/tpetra/core/src.

One note is that Zoltan2's Hypergraph algorithm does explicitly using CrsMatrix, so that would have to be disabled if we go down this path. @egboman comments?

This issue does not occur in the same way for Ifpack2 or MueLu since they process ETI differently and don't do GO/int-as-scalar.

egboman commented 2 months ago

@csiefer2 Where do you see Zoltan2 use Tpetra CrsMatrix with integer types? I thought Zoltan2 just calls old Zoltan to use PHG, and old Zoltan does not use Tpetra or Epetra.

cgcgcg commented 2 months ago

@egboman Maybe this is what @csiefer2 had in mind? https://github.com/trilinos/Trilinos/blob/ee6599d413f7ef5786a5ad1f719bbfe3a4d22300/packages/zoltan2/core/CMakeLists.txt#L38-L50

egboman commented 2 months ago

Maybe, but what does this have to do with hypergraphs?

cgcgcg commented 2 months ago

https://github.com/trilinos/Trilinos/blob/ee6599d413f7ef5786a5ad1f719bbfe3a4d22300/packages/zoltan2/core/src/models/Zoltan2_HyperGraphModel.hpp#L540 https://github.com/trilinos/Trilinos/blob/ee6599d413f7ef5786a5ad1f719bbfe3a4d22300/packages/zoltan2/core/src/models/Zoltan2_ModelHelpers.hpp#L60-L64

csiefer2 commented 2 months ago

@egboman So would you complain if we added logic to Zoltan2 to disable hypergraph if Tpetra doesn't have matrices with SC=int ?

egboman commented 2 months ago

@csiefer2 Maybe. This needs to be done carefully. What if somebody actually wants to use Zoltan2 with hypergraph partitioning but has no clue about Tpetra and scalar types? How would they know to build Trilinos with SC=int? Would they get an error at compile time or run time?

csiefer2 commented 2 months ago

@egboman

I can think of a few options: 1) We could leave Tpetra building with SC=int as the default and then have a runtime error if you try to use Zoltan2 hypergraph partitioning if you turn it off. 2) We can add an explicit on-by-default option to enable Zoltan2's hypergraph partitioning. if you turn off Tpetra SC=int and try to turn Zoltan2's hypergraph on, you get a configure error. 3)We could also modify Zoltan2 to use SC=GO instead of int. Short term, that would work fine, but I would like to enable users to not have SC=GO/int matrices / graphs if they don't want that. So this isn't my favorite options.

egboman commented 2 months ago

@csiefer2

I think (1) is the safest option, but might not help you much reduce compilation time. Option (2) also seems OK but adds another option.

csiefer2 commented 2 months ago

@egboman Ignore the Tpetra stuff, but are the Zoltan2 changes reasonable? This is Option #2 (I couldn't see a good way of making Option #1 work when I tried)

https://github.com/trilinos/Trilinos/tree/csiefer-tpetra-zoltan2-reduced-eti

egboman commented 2 months ago

Yes, looks good to me! Note a typo "wwhen" in Zoltan_config.h :)