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

Absorption #726

Merged
merged 6 commits into from
Feb 2, 2022
Merged

Absorption #726

merged 6 commits into from
Feb 2, 2022

Conversation

yang-ruoxi
Copy link
Contributor

@yang-ruoxi yang-ruoxi commented Dec 8, 2021

Summary

  • Added AbsorptionFW to run a frequency-dependent dielectric calculation, with two options: independent-particle approximation (IPA) and random-phase approximation (RPA). One can also specify a static run, but it will invoke the MPStaticSet rather than the AbsorptionSet.
  • Added absorption task to write input sets for these runs
  • Added a section in vasp drones to collect the dielectrics information from RPA calculation, and to calculate the corresponding absorption coefficient for easy access.
  • A typical usage of this FW would be :
fw0 = OptimizeFW(structure)
fw1 = AbsorptionFW(mode = "STATIC", parent = fw0) # uses MPStaticSet
fw2 = AbsorptionFW(mode = "IPA", parent = fw1)   # uses AbsorptionSet supplied in pymatgen 
fw3 = AbsorptionFW(mode = "RPA", parent = fw2)  # uses AbsorptionSet supplied in pymatgen 
wf = Workflow([fw0, fw1, fw2, fw3])

@yang-ruoxi yang-ruoxi marked this pull request as ready for review December 10, 2021 00:05
@itsduowang
Copy link
Contributor

Hello @yang-ruoxi I think now you can run the workflows to test this PR. Please let me know for any problems or concerns.

Thanks for your contribution. Cheers! :)

@janosh
Copy link
Member

janosh commented Feb 1, 2022

Would be cool to get this merged!

@itsshawnzhang If @yang-ruoxi enabled edits by maintainers, you should be able to push a small change to this PR yourself and trigger CI that way.

@itsduowang
Copy link
Contributor

itsduowang commented Feb 1, 2022

Would be cool to get this merged!

@itsshawnzhang If @yang-ruoxi enabled edits by maintainers, you should be able to push a small change to this PR yourself and trigger CI that way.

Thanks @janosh and @yang-ruoxi. I will push some changes to this PR and run the new CI at this point.

@janosh
Copy link
Member

janosh commented Feb 1, 2022

@itsshawnzhang Is allow edits from maintainers disabled? If so, it might be easier to add a the workflow_dispatch trigger to our test.yml GHA. I think that should allow you to trigger CI through a button and would avoid the need to create a duplicate PR.

workflow-dispatch-inputs

@itsduowang
Copy link
Contributor

@itsshawnzhang Is allow edits from maintainers disabled? If so, it might be easier to add a the workflow_dispatch trigger to our test.yml GHA. I think that should allow you to trigger CI through a button and would avoid the need to create a duplicate PR.

workflow-dispatch-inputs

Will try this! Thanks!

@janosh
Copy link
Member

janosh commented Feb 1, 2022

@itsshawnzhang This might be helpful.

- uses: actions/checkout@v2
  with:
    ref: pull/${{ github.event.pull_request.id }}/head

If you want to manually dispatch a Workflow for a Pull Request you could use workflow_dispatch with an input of the Pull Request id that the Workflow then composes the ref from (in the format above).

@itsduowang
Copy link
Contributor

This is helpful. Please allow me to look into it and I will create the check workflow for this PR. Thanks so much for the information @janosh :)

@yang-ruoxi
Copy link
Contributor Author

Thanks @janosh and @yang-ruoxi. I will push some changes to this PR and run the new CI at this point.

Thanks for looking into it! Is there anything I should do from my end to merge it?

@janosh
Copy link
Member

janosh commented Feb 2, 2022

@yang-ruoxi If you could push any change to this PR (fix a typo or improve a doc string or something like that) to retrigger CI that would allow @itsshawnzhang to make sure it passes all tests before merging.

@janosh
Copy link
Member

janosh commented Feb 2, 2022

@yang-ruoxi The failing tests are due to pymongo v3/v4 incompatibilities. Can you merge or rebase the main branch into your PR?

@itsduowang itsduowang merged commit 2c91190 into hackingmaterials:main Feb 2, 2022
@itsduowang
Copy link
Contributor

Hello @yang-ruoxi . I merged this branch since all the tests are passed in the new environmental settings (see #744). Thank you so much for your contributions. Please let us know for any other problems or concerns. Cheers! : )

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