-
Notifications
You must be signed in to change notification settings - Fork 31
Add int64 variant to package.py
#1553
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
base: develop
Are you sure you want to change the base?
Conversation
kennyweiss
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding this variant @ebchin
I wonder if there's a better name we could use for this though (but nothing comes immediately to mind). I checked the hypre package, and they use int64 for an equivalent option, so I suppose we should go with this.
|
Side question for axom team -- we previously discussed setting the default IndexType to 64-bit. Would this be a good time to do this? E.g. by making the default spack variant |
|
Yeah, the |
|
@kennyweiss I think changing the default index type in Axom to 64-bit int is a good idea. It would probably prevent some user issues and others can set it to a lower resolution type if they want/need to. |
|
Thanks @rhornung67. In that case, @ebchin -- could you please change the default for (we'll change the default CMake option on the axom side in a follow-up PR). |
Previously, we had an explicit 64-bit IndexType run.
3017485 to
ff99cd4
Compare
|
|
||
| // 3D Elvira makes polyhedral meshes | ||
| auto srcTopologyView = views::make_unstructured_polyhedral_topology<int>::view(n_src_topology); | ||
| auto srcTopologyView = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@BradWhitlock -- this was failing w/ axom::IndexType == std::int64_t. Change the template to axom::IndexType fixed it and seems to be the right thing to do.
Summary
AXOM_USE_64BIT_INDEXTYPEthrough the spack variantint64AXOM_USE_64BIT_INDEXTYPEby default (see Issues related toaxom::IndexType#718). As suchaxom::IndexTypewill default tostd::int64_tinstead ofstd::int32_t.Context
Recently, we've been running large problems in Tribol which have needed this option. Exposing it to the spack recipe will simplify enabling this in our TPL builds.