Closed wladerer closed 1 year ago
I've run a test case with the implementation, i will create a fork and submit a pull request
Hello @wladerer ,
Thanks a lot for your contribution! We will look at your pull request as soon as possible. One first comment I would make is to make a unit test and possibly an integration test. This is essential to make sure all the features continue to work in the future. This may be difficult for you to dig in but you can probably do something similar to what is done for the other inputs as well, and as you have run a test case, you can use that one for the tests. Do not hesitate to ask if you have issues. Documentation on how to implement unit tests for the DefineRunner are found here: https://matgenix.github.io/turbomoleio/developer/definerunner.html#tests And general documentation on how to implement unit and integration tests are found here: https://matgenix.github.io/turbomoleio/developer/testing.html When this is done you can also add your feature to the documentation.
About your potential issue, I would need to dig a little bit more what could be done to prevent cases where the user provides atoms that are not found. We will see about that when reviewing.
One last point (which is not strictly needed at this stage and I would consider merging without that of course), when running with specific core potentials, are there additional outputs in the log files that could/should be parsed ? In that case, it would be interesting to also add this in the output parsing module. Again, this is not essential but may be interesting to "close the loop".
Best,
David
Hi, my pleasure!! This package has made my life tremendously better and I want to help when I can. I'm not the most experienced developer, just a chemist with some spare time. I will go about running the unit and integration tests soon.
For the "atom not found" we could perhaps catch that ourselves? If we were able to sift through the coord file or other inputs to check if that atom exists in the first place? The other thing is that the ecp nicknames are quite fickle, so it might be better to treat the error message, rather than anticipating input errors. I will think on that.
As for closing the loop, the only thing I can really think of is how the control file would output the basis set for each atom + the core potential below that entry. I will sift through some of my previous jobs to see if changing the core potential has any signature in the other output files.
Ok, so I was unable to find a runtest.sh script in the project files. Instead, I ran
pytest --html=~/Desktop/report.html
In the root directory of the project. I hope this is an acceptable method.
I am using TURBOMOLE V6.6 from 2014, for reference.
Attached is the PDF of the report, and I will paste a google drive link to the HTML document:
https://drive.google.com/file/d/1V-Wk_T7RU9oaYlXE0Qp1TmhadwAsneTY/view?usp=sharing
Hi everyone,
I wanted to be able to define effective core potentials on specific atoms. I'm not the most experienced programmer, so I do not claim to have done this in the most effective/safe way possible. My implementation is almost identical to the
_set_basis
and_define_basis_sets
methods:This would then necessitate updating the
run_full
function as follows:One potential issue: When I run
define
through the command line interface and I try to apply an ecp to an atom that is not found in the coord file, there is no error message, it just passes nothing. I hope that this wont create an infinite loop.Let me know if I can elaborate or make any of my points more clear. Thank you!