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

Update Trilinos configuration for complex & float. #375

Merged
merged 1 commit into from
May 16, 2024

Conversation

marcfehling
Copy link
Member

@marcfehling marcfehling commented Feb 17, 2024

In reference to trilinos/Trilinos#12750.

See also: https://www.docs.trilinos.org/files/TrilinosBuildReference.html#enabling-float-and-complex-scalar-types

I moved the ENABLE_FLOAT option to the full configuration of Trilinos, as opposed to just for Teuchos. Is there a reason you only enabled it for Teuchos?

@@ -263,9 +263,7 @@ CONFOPTS="\

if [ ${TRILINOS_WITH_COMPLEX} = ON ]; then
CONFOPTS="\
-D Trilinos_ENABLE_COMPLEX_DOUBLE=ON \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do these two lines not matter?

Copy link
Member Author

@marcfehling marcfehling Feb 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are set to ON dependent on ENABLE_COMPLEX and ENABLE_FLOAT. See also https://www.docs.trilinos.org/files/TrilinosBuildReference.html#enabling-float-and-complex-scalar-types

Long story short: Packages do not use these configuration options consistently in Trilinos, or they apply different logic on how to configure based on these options. Since ENABLE_COMPLEX and ENABLE_FLOAT also set ENABLE_COMPLEX_DOUBLE and ENABLE_COMPLEX_FLOAT, I believe it is more robust to go with the first set of options.

@marcfehling
Copy link
Member Author

How do we proceed here?

@tjhei tjhei merged commit 75f80f0 into master May 16, 2024
6 of 10 checks passed
@tjhei
Copy link
Member

tjhei commented May 16, 2024

Sorry, thank you for digging into this issue. I will merge and then we will see if any problems come up.

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

Successfully merging this pull request may close these issues.

2 participants