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 cell optimization to levelparser. #33

Merged
merged 2 commits into from
Dec 8, 2023

Conversation

hdsassnick
Copy link

Hello,

first off, thank you for developing and maintaining such a nice parser for CP2K.

For my use cases I would need to parse more information on cell optimization calculations. As a first step, I generalized the geometry optimization part of the code to also work for the cell optimization case.

I would be very grateful for your feedback on the pull request and would be happy to implement improvements or tests for the new functionality.

All the best,
Holger

@hdsassnick
Copy link
Author

Hi,

just wanted to give a quick reminder and ask whether there is anything that can be done to speed up the process of implementing the contribution.

Thank you and all the best,
Holger

@dev-zero
Copy link
Contributor

dev-zero commented Dec 6, 2023

Hi Holger,

sorry for the delay. It would be great if you could add a minimal output sample to the tests/outputs together with a small test. If you do it similar to for example the https://github.com/cp2k/cp2k-output-tools/blob/develop/tests/test_condition_number.py test, interested users will then also see the resulting structure.
Also, if you could check that the existing tests are still passing? For some reason GitHub did not run them on your PR.

Thanks a lot in advance,
Best,
Tiziano

@hdsassnick
Copy link
Author

Hi Tiziano,

I just added a test for the levelparser optimization. I had to vary the structure of the test a bit compared to the example that you gave since the optimization blocks are used for the levelparser.

I am very happy to receive your feedback. All tests run fine on my local installation.

All the best,
Holger

@dev-zero dev-zero merged commit ecea8ad into cp2k:develop Dec 8, 2023
4 checks passed
@dev-zero
Copy link
Contributor

dev-zero commented Dec 8, 2023

Hi Holger,

awesome, thanks a lot!

Merged, and figured out why the GHA did not run: it seems GH has changed the policy that an external contributor's first PR must be explicitly permitted. All green.

Best,
Tiziano

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.

2 participants