atomate2
atomate2 copied to clipboard
[WIP] add draft for gruneisen workflow
Summary
This PR aims to add a Gruneisen workflow using phonopy API
Todo
- [ ] Add job to compute Gruneisen parameter to the flow
- [ ] Add tests
- [ ] Add a VASP-compatible flow as well (currently only forcefield flow is added)
Hi @JaGeo, @joabustamante and @kaueltzen, just tagging you all here for updates.
Codecov Report
Attention: Patch coverage is 89.00524%
with 21 lines
in your changes missing coverage. Please review.
Project coverage is 74.97%. Comparing base (
29a5731
) to head (580312e
). Report is 40 commits behind head on main.
:exclamation: Current head 580312e differs from pull request most recent head e8a77ba
Please upload reports for the commit e8a77ba to get more accurate results.
Additional details and impacted files
@@ Coverage Diff @@
## main #752 +/- ##
==========================================
+ Coverage 74.94% 74.97% +0.03%
==========================================
Files 136 144 +8
Lines 10513 11018 +505
Branches 1643 1694 +51
==========================================
+ Hits 7879 8261 +382
- Misses 2143 2252 +109
- Partials 491 505 +14
Files | Coverage Δ | |
---|---|---|
src/atomate2/common/schemas/phonons.py | 93.63% <100.00%> (-0.45%) |
:arrow_down: |
src/atomate2/common/schemas/gruneisen.py | 96.29% <96.29%> (ø) |
|
src/atomate2/forcefields/flows/gruneisen.py | 82.60% <82.60%> (ø) |
|
src/atomate2/common/jobs/gruneisen.py | 88.67% <88.67%> (ø) |
|
src/atomate2/common/flows/gruneisen.py | 83.33% <83.33%> (ø) |
Thanks!
Can we add that we might need to adapt the current optimization in the force fields, as we don't change the cell shape currently and that we need symmetry and potentially checks that the optimization has converged (especially for force fields?)
If you also want to make this more of a common workflow I can add aims support
@tpurcell90 Surely. It will take some time though. The current Implementation is the result of a first tiny hackathon that we had to introduce jobflow concepts more widely in the group
Ya no rush on that, just ping me when you want me to take a look at it
Hi @JaGeo, We have updated the job to the flow that also includes computing Gruneisen parameters from the phonon runs.
Next, we will add a schema for the flow if desired and also tests
Thank you both, @joabustamante and @naik-aakash !
I left one inital comment. Might need to be checked in the schema itself.
@naik-aakash @joabustamante Great work already! :)
Hi @tpurcell90, We have now added a draft for common flow as BaseGruneisenMaker
. It would be great if you could have a look at it and provide us feedback on it, mainly whether it would be fine to use it with FHI-aims in its current state. And also if you have suggestions for the output schema. We have added very minimal schema as of now.
The custom bandstructure plotter in the schema will be moved to Pymatgen in the next few days (it's residing here now just for testing purposes)
We plan to add a VASP-compatible flow soon, but it could take some time.
Hi @tpurcell90, We have now added a draft for common flow as
BaseGruneisenMaker
. It would be great if you could have a look at it and provide us feedback on it, mainly whether it would be fine to use it with FHI-aims in its current state. And also if you have suggestions for the output schema. We have added very minimal schema as of now.The custom bandstructure plotter in the schema will be moved to Pymatgen in the next few days (it's residing here now just for testing purposes)
We plan to add a VASP-compatible flow soon, but it could take some time.
Ya I can do this, I will try to get this done next week (just flew into Berlin for a visit and have meetings all day today)
I will now try to implement the VASP support.
Hi @tpurcell90, We have now added a draft for common flow as
BaseGruneisenMaker
. It would be great if you could have a look at it and provide us feedback on it, mainly whether it would be fine to use it with FHI-aims in its current state. And also if you have suggestions for the output schema. We have added very minimal schema as of now. The custom bandstructure plotter in the schema will be moved to Pymatgen in the next few days (it's residing here now just for testing purposes) We plan to add a VASP-compatible flow soon, but it could take some time.Ya I can do this, I will try to get this done next week (just flew into Berlin for a visit and have meetings all day today)
There is nothing obvious that would prevent FHI-aims from also having this, but I will try to implement it now
@tpurcell90 I started working on the vasp integration. I have already done some small modifications. Maybe, let me push that first. We need different names for the jobs before we can implement any tests
@tpurcell90 I started working on the vasp integration. I have already done some small modifications. Maybe, let me push that first. We need different names for the jobs before we can implement any tests
Okay I will wait for you to be done first and then port it for FHI-aims
@utf should be ready for review. I have now also added tests for the VASP parts.
@jmmshn Just to notify you: the job names of your workflows have slightly changed due to an update on jobflow. I hope this does not affect any of your analysis scripts.
Tests are finally passing.
@jmmshn Just to notify you: the job names of your workflows have slightly changed due to an update on jobflow. I hope this does not affect any of your analysis scripts.
Can you point me to the jobflow PR for this?
@jmmshn Just to notify you: the job names of your workflows have slightly changed due to an update on jobflow. I hope this does not affect any of your analysis scripts.
Can you point me to the jobflow PR for this?
Yes, sure. I am also responsible for this one as it was a requirement for testing any more complex workflows. https://github.com/materialsproject/jobflow/pull/644
I am happy to help to fix the naming for both of your workflows but it is likely faster if you point me to the parts where a name might be appended to a response object in the response object.
@utf I think I implemented all suggestions. Let me know what you think.
@utf thank you for the comments. I will correct the broken sentences in the comments and then wait for your opinion on the settings
@utf should hopefully now be ready to be merged. I had to adapt the code after the new input sets got merged.
Hi @JaGeo , It would be better to now replace the method used for generating Gruneisen bandstrucuture plot in the schema to use the one we implemented in pymatgen, before merging this PR. I will push the change if it is fine with you in sometime today 😃
Hi @JaGeo , It would be better to now replace the method used for generating Gruneisen bandstrucuture plot in the schema to use the one we implemented in pymatgen, before merging this PR. I will push the change if it is fine with you in sometime today 😃
Nevermind, seems there are other workflows breaking on updating pymatgen version. So I leave it as is for now and change it later.
okay, @naik-aakash !
@utf this would be ready to merge!
This looks great. Thanks @JaGeo @naik-aakash and all other contributors.