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

Nexus: expand eshdf features #3334

Merged
merged 3 commits into from
Aug 25, 2021
Merged

Nexus: expand eshdf features #3334

merged 3 commits into from
Aug 25, 2021

Conversation

Paul-St-Young
Copy link
Contributor

Proposed changes

No change to default behavior. Options added:

  1. --fnk3d outputs the 3D n(k) to the specified hdf file
  2. --ispin=0,1 targets spin channel
  3. --ncore and --nval define ``active'' orbitals that contribute to n(k)

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?

workstation, Xeon 6234, CentOS 7

Checklist

Update the following with a yes where the items apply. If you're unsure about any of them, don't hesitate to ask. This is
simply a reminder of what we are going to look for before merging your code.

  • Yes. This PR is up to date with current the current state of 'develop'
  • No. Code added or changed in the PR has been clang-formatted
  • No. This PR adds tests to cover any new code, or to catch a bug that is being fixed
  • No. Documentation has been added (if appropriate)

no change to default behavior. Options added:

1. --fnk3d outputs the 3D n(k) to the specified hdf file
2. --ispin=0,1 targets spin channel
3. --ncore and --nval define ``active'' orbitals that contribute to n(k)
@qmc-robot
Copy link

Can one of the admins verify this patch?

@jtkrogel
Copy link
Contributor

jtkrogel commented Aug 4, 2021

The functionality here looks good. The intended usage of eshdf is eshdf [mode] [options]. Please can you move the new functionality to a new function at the same level as kinetic (e.g. write_nk) that will process only what is needed for n(k) according to the new options you introduce? Invocation would then look like e.g. eshdf write_nk [options].

nexus/bin/eshdf Outdated Show resolved Hide resolved
nexus/bin/eshdf Outdated Show resolved Hide resolved
nexus/bin/eshdf Show resolved Hide resolved
@Paul-St-Young
Copy link
Contributor Author

@jtkrogel I split out the write_nk subcommand like you suggested.
I also pulled out two option parsing subroutines from kinetic() to reuse in write_nk()

  1. first_eshdf_file
  2. interpret_nk_options

Please see if this PR is now satisfactory.

@jtkrogel
Copy link
Contributor

@Paul-St-Young This looks OK now. Please can you just quickly confirm that running eshdf kinetic ... produces the same output as before?

@Paul-St-Young
Copy link
Contributor Author

@jtkrogel yes, I can confirm eshdf kinetic works the same as before on my example wf h5 file.

Copy link
Contributor

@jtkrogel jtkrogel left a comment

Choose a reason for hiding this comment

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

LGTM

@ye-luo
Copy link
Contributor

ye-luo commented Aug 25, 2021

Test this please

@ye-luo ye-luo enabled auto-merge August 25, 2021 19:06
@ye-luo ye-luo merged commit 0b2dd7a into QMCPACK:develop Aug 25, 2021
@Paul-St-Young Paul-St-Young deleted the eshdf branch July 5, 2022 20:32
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