Skip to content

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

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

Add int64 variant to package.py #1553

wants to merge 3 commits into from

Conversation

ebchin
Copy link
Member

@ebchin ebchin commented Apr 17, 2025

Summary

  • This PR is a small addition to the axom spack recipe.
  • It does the following:
    • Enables setting AXOM_USE_64BIT_INDEXTYPE through the spack variant int64

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.

@ebchin ebchin requested review from white238 and kennyweiss April 17, 2025 17:01
Copy link
Member

@kennyweiss kennyweiss left a 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.

See: https://github.com/spack/spack/blob/f0a7388496a5cb062d7ec6541273b6cc697ba051/var/spack/repos/builtin/packages/hypre/package.py#L65

@kennyweiss
Copy link
Member

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 +int64?
See: #718

@ebchin
Copy link
Member Author

ebchin commented Apr 17, 2025

Yeah, the int64 name came from hypre. Happy to change if there's a consensus on something better.

@rhornung67
Copy link
Member

@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.

@kennyweiss
Copy link
Member

kennyweiss commented Apr 17, 2025

Thanks @rhornung67.

In that case, @ebchin -- could you please change the default for int64 to True?

(we'll change the default CMake option on the axom side in a follow-up PR).

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

Successfully merging this pull request may close these issues.

3 participants