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

Add ValueAlias for ValueType reconstruction #4825

Merged
merged 4 commits into from
Nov 11, 2023

Conversation

ye-luo
Copy link
Contributor

@ye-luo ye-luo commented Nov 9, 2023

Proposed changes

Add a helper class for Value type reconstruction.

What type(s) of changes does this code introduce?

  • New feature

Does this introduce a breaking change?

  • No

What systems has this change been tested on?

laptop

Checklist

  • Yes. This PR is up to date with current the current state of 'develop'
  • Yes. Code added or changed in the PR has been clang-formatted

@ye-luo ye-luo requested a review from PDoakORNL November 9, 2023 22:59
Copy link
Contributor

@PDoakORNL PDoakORNL left a comment

Choose a reason for hiding this comment

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

I don't see a problem with this but I guess I don't understand why its needed.

I always have assumed that classes and functions would be templated on Value since that basically determines everything except what the full precisions is.
If a derived type is complex definite and you have an indeterminant type I always assumed its type would be determined via

std::complex<RealAlias<T>> var;
// or
std::complex<FullPrecReal> var_full_prec;

What am I not seeing here?

@ye-luo
Copy link
Contributor Author

ye-luo commented Nov 10, 2023

I don't see a problem with this but I guess I don't understand why its needed.

I always have assumed that classes and functions would be templated on Value since that basically determines everything except what the full precisions is. If a derived type is complex definite and you have an indeterminant type I always assumed its type would be determined via

std::complex<RealAlias<T>> var;
// or
std::complex<FullPrecReal> var_full_prec;

What am I not seeing here?

Value is not complex definite. QTBase and QTFull relies on ifdef. To have full template, I tries not ot rely on QTFull but promote precision but to derive FullPrecValue based on Value using ValueAlias.

@PDoakORNL
Copy link
Contributor

This is why I thought we would need Value and FullPrecValue or FullPrecReal for value/precision templating. I'm unclear on how if you have only a Real or a Value type coming into a function how you can discover whether it is reduced or full precision and what the full precision is without some global type alias.

This is why WaveFunctionTypes.hpp looks like it does, but I'll wait and see how this is used.

@ye-luo
Copy link
Contributor Author

ye-luo commented Nov 10, 2023

WaveFunctionTypes<VALUE, VALUE_FP> needs to consistently put value types. If one use WaveFunctionTypes<VALUE, REAL_FP> it breaks. With added reconstruct capability, it can be changed to WaveFunctionTypes<VALUE, FP_PREC> and internally derive VALUE_FP as ValueAlias<FP_PREC, VALUE>.

@PDoakORNL
Copy link
Contributor

I get you now. I agree that <VALUE, FP_PREC> is better. And now I see what you are after here.

@prckent
Copy link
Contributor

prckent commented Nov 10, 2023

Test this please

@prckent prckent enabled auto-merge November 10, 2023 22:58
@prckent prckent merged commit 470e00c into QMCPACK:develop Nov 11, 2023
@ye-luo ye-luo deleted the value-alias branch November 11, 2023 00:05
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.

3 participants