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

64-bit-indices #163

Open
minrk opened this issue Jun 29, 2023 · 16 comments
Open

64-bit-indices #163

minrk opened this issue Jun 29, 2023 · 16 comments
Labels

Comments

@minrk
Copy link
Member

minrk commented Jun 29, 2023

Comment:

It's recently come up that folks would like builds of petsc with 64-bit indices enabled.

I don't know enough to know whether there's a significant cost to having this enabled by default. If we do enable it by default, does this make an ABI change that would cause a compatibility problem (i.e. we would need to try to avoid installing the new builds with packages built against the old one)?

If this is a niche case or a pain, I'm okay with the answer being "pick a different distribution", I just don't know enough to give that answer.

Do you have any comment on this @dalcinl?

@minrk minrk added the question label Jun 29, 2023
@minrk
Copy link
Member Author

minrk commented Jun 29, 2023

If it's a good idea but the compatibility is a pain, one way to do it is to apply the change with the first 3.20 build.

@dalcinl
Copy link
Contributor

dalcinl commented Jun 30, 2023

If the decision were on my hands, I would have switched PETSc to build with 64bit indices by default long ago.
However, as PETSc still defaults to 32bit indices, I suspect that many third-party codes that use PETSc may experience issues with a switch to 64bit. Python packages using PETSc via petsc4py may be mostly unaffected thanks to the numpy layer, but for C/C++ codes, things can go wrong very quickly.

I would argue that a switch to 64bit would require PETSc switching to 64bit by default, and then, after a couple of release cycles, we make the switch in conda-forge.
CC @BarrySmith @balay @stefanozampini

In the meantime, what if we follow Debian with a petsc64 package?

@BarrySmith
Copy link

I would be surprised if more than a small minority of PETSc users truly needed 64 bit indices, but making it the default and moving away from 32 bit indices completely certainly simplifies some configuration and packaging issues.

@dalcinl
Copy link
Contributor

dalcinl commented Jul 3, 2023

I would be surprised if more than a small minority of PETSc users truly needed 64 bit indices,

You have a point, maybe the majority only have to deal with toy problems (by toy I mean small in terms of size).
The more important question is how much users lose by using 64bit in problems that do not require it. Memory consumption may be slightly higher. Some performance-critical kernels (e.g. MatMult) may run slower (though PETSc could internally use 32bit ints for small matrices). Ultimately, perhaps we should regards 32bit indices as a performance option users have to opt-in for problems that can afford it, pretty much as we currently view single precision scalars.

@minrk
Copy link
Member Author

minrk commented Jul 3, 2023

Is switching the default an option for 3.20? I do like the reasoning of "(small?) performance impact" vs "doesn't work at all" for folks who are faced with a PETSc not built with the right flag for them.

@dalcinl
Copy link
Contributor

dalcinl commented Jul 3, 2023

"doesn't work at all"

Not to mention that the "failure mode" is not always obvious, and users may have to spend quite a bit of time figuring out the problem.

@stefanozampini
Copy link
Contributor

stefanozampini commented Jul 3, 2023

"doesn't work at all"

Not to mention that the "failure mode" is not always obvious, and users may have to spend quite a bit of time figuring out the problem.

If I may say something here, this specific problem has popped up by trying to solve a problem with a huge mesh. There should be a limit between something that can be automated and should "just work", and something that instead necessitates users "to use their brain". Next question would then be: why my direct solver takes years to setup? I think Lisandro already knows my position here.

Also, regarding the error message. Errors would have appeared earlier in the program if one would have used a debug version, but for performance reasons we cannot check everything in optimized mode. The error message in this case should be handled better by fenicsx, not by Petsc

@dalcinl
Copy link
Contributor

dalcinl commented Jul 3, 2023

@stefanozampini I don't quite get what's your point here. Are you opposed to 64bit builds by default?

@stefanozampini
Copy link
Contributor

@stefanozampini I don't quite get what's your point here. Are you opposed to 64bit builds by default?

Yes

@stefanozampini
Copy link
Contributor

I guess a solution would be ti offer customized builds , something like conda install petsc=int==64bit

@dalcinl
Copy link
Contributor

dalcinl commented Jul 3, 2023

The limit between small mesh and large mesh is not crystal clear. This issue has bitten me many times (and even maybe you), with people using 32bits and running scaling studies. We are in 2023, the crapies laptop have multi GB RAM, and things will keep going in that direction.

I repeat and ask it again: why is 64bit indices by default any different that double precision by default? Shouldn't defaults be those that accommodate the majority of use cases, even at the price of leaving some performance in the table? Otherwise, why don't you advocate for switching back to PETSc building with static libraries by default, or using single precision by default? Shouldn't generality trump? Why do you think numpy's default integral type is 64bit on 64bit architectures? Why don't you like 64bit integers by default? What's your real concern?

necessitates users "to use their brain"

While I agree with your statement, I don't consider this particular issue one that requires "brain use". Even if you figure immediately what's the problem, many times you are stuck with the 32bit build and that's not always easy to change.

I guess a solution would be ti offer customized builds , something like conda install petsc=int==64bit

Just because it pleases your preferences, it does not mean it is the proper solution. Besides, we already have a large build matrix in conda-forge.

@minrk
Copy link
Member Author

minrk commented Jul 3, 2023

If there's a strong reason to have both (e.g. the performance cost is high and the number of users affected is small), then a build variant is the way to go (that would mean 20 more petsc builds and 112 more petsc4py builds). We'll just need to make sure they signal their incompatibility, assuming they are not ABI compatible. This could be done by adding it to the scalar prefix, e.g. replacing the current real_* with real64_* and real32_*. That would work today, since those would both be rejected by the current run_exports.

Another option is to say that it's out of scope for the conda packages to have this build variant, and folks solving large problems must use source/spack/containers/etc. instead of conda. There's a good chance folks working on large problems should have a more optimized build than we provide anyway. Which, again, could be appropriate if the number of affected users is small and the cost is high.

@dalcinl
Copy link
Contributor

dalcinl commented Jul 3, 2023

assuming they are not ABI compatible.

The are definitely not ABI compatible.

replacing the current real_* with real64_* and real32_*

That particular choice would be confusing, real32/64 may be interpreted as single/double precision.

@BarrySmith
Copy link

"doesn't work at all"

Not to mention that the "failure mode" is not always obvious, and users may have to spend quite a bit of time figuring out the problem.

We should try to add appropriate error checking to help determine the "failure modes" better. So if anyone knows cases where failure occurred but was not obvious let us know and we can study them to improve our error checking.

@BarrySmith
Copy link

If we weren't stuck with the current PETSc C code model, it could be easier to have the libraries be more flexible and automatically handle working with different integer sizes (for example, inside the matrix object automatically switching its integer size as needed; this is difficult because Mat has a very wide interface and the size of integer inside bleeds out in many places so, as Jed, found out, just trying to change a few things inside Mat_AIJ is painful codewise.

@aaschwanden
Copy link

+1 on a 64bit indices version of PETSc. We (developers of the Parallel Ice Sheet Model, https://github.com/pism/pism) are currently working on a PISM conda package. Our applications frequently require PETSc built with 64bit indices support.

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

No branches or pull requests

5 participants