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

Add missing directory kwarg on QCJob run() method #323

Merged
merged 2 commits into from
Mar 12, 2024

Conversation

Andrew-S-Rosen
Copy link
Member

@Andrew-S-Rosen Andrew-S-Rosen commented Mar 11, 2024

Summary

The QCJob class' run() method was missing a directory keyword argument, which is needed in order to have the subprocess call run in the pre-specified directory.

@janosh janosh added fix Bug fix qchem Q-Chem general-purpose electronic structure package labels Mar 12, 2024
Copy link
Member

@janosh janosh left a comment

Choose a reason for hiding this comment

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

would be nice to have a test for QCJob.run() but i assume that's hard/slow?

@janosh janosh merged commit 26b828b into materialsproject:master Mar 12, 2024
8 checks passed
@Andrew-S-Rosen
Copy link
Member Author

Andrew-S-Rosen commented Mar 12, 2024

@janosh: Yeah, it would be nice. I must admit, my familiarity is primarily with the VASP modules, so I'm not sure how viable/not viable it would be to address. I imagine it would involve a fair bit of monkeypatching.

Andrew-S-Rosen added a commit to Quantum-Accelerators/quacc that referenced this pull request Mar 12, 2024
## Summary of Changes

Call Custodian directly rather than via a subprocess for Q-Chem.

Requires:
- materialsproject/custodian#323
- Custodian > 2024.2.15

### Checklist

- [X] I have read the ["Guidelines"
section](https://quantum-accelerators.github.io/quacc/dev/contributing.html#guidelines)
of the contributing guide. Don't lie! 😉
- [X] My PR is on a custom branch and is _not_ named `main`.
- [X] I have added relevant, comprehensive [unit
tests](https://quantum-accelerators.github.io/quacc/dev/contributing.html#unit-tests).

### Notes

- Your PR will likely not be merged without proper and thorough tests.
- If you are an external contributor, you will see a comment from
[@buildbot-princeton](https://github.com/buildbot-princeton). This is
solely for the maintainers.
- When your code is ready for review, ping one of the [active
maintainers](https://quantum-accelerators.github.io/quacc/about/contributors.html#active-maintainers).

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix Bug fix qchem Q-Chem general-purpose electronic structure package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants