-
Notifications
You must be signed in to change notification settings - Fork 8
Added link to the pump pulse HDF5 file #95
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
Conversation
|
Yes, I can take care of it when I get back! |
|
Sounds great, Laurie! Happy holidays! |
|
I am doing some additional minor changes, so let's not merge it right now. |
|
Hey @imaliyov , what should we add here in order to make this PR ready to merge? |
|
Hi! All four updates should come together: perturbo-dev, hotfix on perturbo public, perturbopy, website. I am going to update the tests and, possibly, tutorial examples, then we can merge everything. |
… with dynamics-pp
* fixed bug with the k-grid indexing for transition dipoles * fixed bug with writing the cdyna HDF5 file for the carrier concentration check (cnum_check flag) * more strict requirements for the .to_cdyna method on the array of snapshots shape
893b09e to
617f312
Compare
|
Hi Sergei, would you recheck this pull request as well please? |
|
Thank you all. Please note that once this branch of perturbopy is merged, the perturbo public and perturbo-dev should be also up to date (with the corresponding branches there merged), and ideally the examples updated too. We discussed this with Kelly. |
|
Ok, so all 3 PRs should be merged simultaneously, correct? |
|
Ideally, yes. Definitely, all 3 PRs before the workshop. The old and new versions are not compatible between them, so if at least one is not merged, pump pulse feature will be unusable (other features are unmodified). It is best to agree on PR merge timing with @jyyao27 |
hurricane642
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Link points to the HDF5 file description (in the updated version of the website). On the website, the branch has the same name and PR is created too.
@ltan01 or @hurricane642, could you please generate the interactive workflow with the updated YAML files?
On my system, for some reason, the SVG file of the workflow is smaller size. I don't know why, it worked previously well.
Anyways, for this small change, if someone could generate the HTML file for the workflow, would be great!