Skip to content

Nexus: Add GCTA docs and update examples - #5316

Merged
ye-luo merged 5 commits into
QMCPACK:developfrom
aannabe:gcta_docs
Feb 20, 2025
Merged

ye-luo merged 5 commits into
QMCPACK:developfrom
aannabe:gcta_docs

Conversation

@aannabe

@aannabe aannabe commented Feb 13, 2025

Copy link
Copy Markdown
Contributor

Proposed changes

This PR adds Nexus documentation for GCTA added in #5029. I was not sure where to put it, so it is currently added at the end of "Nexus User Scripts".
Another minor fix in the documentation: error = 1/N -> error 1/sqrt(N) in CLT.

I also updated two Nexus examples, both of which are supposed to work with the now default batched driver (explicitly specified in qmc_shared object as driver = 'batched'). They were crashing due to the double specification of the driver.
Also, the estimator example was initially intended for the batched driver input.

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

  • Documentation and examples

Does this introduce a breaking change?

  • No

What systems has this change been tested on?

Local machine

Checklist

  • Yes. This PR is up to date with the current state of 'develop'
  • Yes. Documentation has been added (if appropriate)

@aannabe aannabe added the nexus label Feb 13, 2025
@aannabe
aannabe requested review from jtkrogel and prckent February 13, 2025 21:42
@aannabe

aannabe commented Feb 20, 2025

Copy link
Copy Markdown
Contributor Author

Is there anything I could fix/improve in this PR? I have received a few questions regarding this Nexus feature, so I thought some documentation is needed. Please let me know.

@prckent

prckent commented Feb 20, 2025

Copy link
Copy Markdown
Contributor

I'll look this afternoon.

@prckent

prckent commented Feb 20, 2025

Copy link
Copy Markdown
Contributor

Test this please

prckent
prckent previously approved these changes Feb 20, 2025

@prckent prckent left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Many thanks Gani. LGTM.

Also appreciate the observables fix.

I took to opportunity to fix ESPRESSO throughout.

Q. Was there a reason other than convenience that you specified walkers_per_rank?

@aannabe

aannabe commented Feb 20, 2025

Copy link
Copy Markdown
Contributor Author

Q. Was there a reason other than convenience that you specified walkers_per_rank?

Yes:

  1. Making it clear at a glance that this is a "batched" example.
  2. Increase the throughput on GPUs (default is walkers_per_rank = 1).

@prckent prckent left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

(Jaron reports PR looks good now via backchannel)

@prckent

prckent commented Feb 20, 2025

Copy link
Copy Markdown
Contributor

Test this please

@ye-luo
ye-luo enabled auto-merge February 20, 2025 21:29
@ye-luo
ye-luo merged commit 1912b1b into QMCPACK:develop Feb 20, 2025
@aannabe
aannabe deleted the gcta_docs branch February 20, 2025 21:57
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.

4 participants