Skip to content

Introduce global Lattice type#5351

Merged
ye-luo merged 7 commits intoQMCPACK:developfrom
ye-luo:introduce-global-Lattice
Mar 3, 2025
Merged

Introduce global Lattice type#5351
ye-luo merged 7 commits intoQMCPACK:developfrom
ye-luo:introduce-global-Lattice

Conversation

@ye-luo
Copy link
Contributor

@ye-luo ye-luo commented Mar 2, 2025

Proposed changes

Although the code seems to allow different places to define its own CrystalLattice<T> type. At the end of day, all of PtclOnLatticeTraits::ParticleLayout ParticleSet::ParticleLayout effectively refer to CrystalLattice<OHMMS_PRECISION, OHMMS_DIM>. This PR define a global type Lattice as a replacement of all the above and I will eventually change it to CrystalLattice<OHMMS_PRECISION_FULL, OHMMS_DIM> in #5350.

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

  • Refactoring (no functional changes, no api changes)

Does this introduce a breaking change?

  • No

What systems has this change been tested on?

epyc-server

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 marked this pull request as draft March 2, 2025 02:05
@ye-luo
Copy link
Contributor Author

ye-luo commented Mar 2, 2025

Test this please

@prckent
Copy link
Contributor

prckent commented Mar 3, 2025

I'll wait to review this when it is no longer marked as draft, but I am very much on board with the change.
It will take a couple of PRs to get all the changes in , but the one thing I would like to see is a before/after of our full set of performance ctest results on at least CPU (NiO bulk, graphene and C molecule). Any degradation should be minimal. Anything severe would indicate we have to, e.g., update Coulomb handling to convert to float more promptly.

@ye-luo ye-luo marked this pull request as ready for review March 3, 2025 20:00
@PDoakORNL
Copy link
Contributor

test this please

Copy link
Contributor

@prckent prckent left a comment

Choose a reason for hiding this comment

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

LGTM

Definitely an improvement, imo. Expressions like "using ParticleLayout = Lattice;" read oddly currently. Renaming &/or more simplification possible in future.

@PDoakORNL
Copy link
Contributor

test this please

@ye-luo ye-luo merged commit 3185c65 into QMCPACK:develop Mar 3, 2025
37 of 38 checks passed
@ye-luo ye-luo deleted the introduce-global-Lattice branch March 3, 2025 22:31
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