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

Allows writing of Turbomole format with angstrom. #62

Merged
merged 4 commits into from
Dec 4, 2023

Conversation

MtoLStoN
Copy link
Member

@MtoLStoN MtoLStoN commented Dec 1, 2023

At the moment, mctc-lib allows reading of coord files in angstrom format ($coord angs), but can not write these files. With this PR, mctc-lib will write $coord angs files if specified in the input.

Copy link

codecov bot commented Dec 1, 2023

Codecov Report

Attention: 21 lines in your changes are missing coverage. Please review.

Comparison is base (305eb7a) 69.81% compared to head (c8fafbb) 69.79%.

Files Patch % Lines
test/test_write_turbomole.f90 56.09% 0 Missing and 18 partials ⚠️
src/mctc/io/write/turbomole.f90 78.57% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #62      +/-   ##
==========================================
- Coverage   69.81%   69.79%   -0.03%     
==========================================
  Files          64       64              
  Lines        8568     8618      +50     
  Branches     2552     2579      +27     
==========================================
+ Hits         5982     6015      +33     
+ Misses        784      782       -2     
- Partials     1802     1821      +19     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: MtoLStoN <[email protected]>
@MtoLStoN MtoLStoN requested a review from Albkat December 1, 2023 11:29
Copy link
Member

@awvwgk awvwgk left a comment

Choose a reason for hiding this comment

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

Best would be to store the conversation factor in a separate variable and keep most of the code for writing the data groups the same. This avoids creating unnecessary code duplication.

@MtoLStoN
Copy link
Member Author

MtoLStoN commented Dec 1, 2023

You are right, I thought about that too. However, there are a few additional things that differ between writing a normal coord and a coord file in angstrom form, other than just the conversion ($coord angs, and $lattice angs). This could be of course easily done with if clauses, but this would make the code a little bit harder to read. I therefore opted to do it this way in favor of code clarity.

But we can also do it with fewer lines, if you prefer that.

Copy link
Member

@awvwgk awvwgk left a comment

Choose a reason for hiding this comment

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

Looks good to me now.

@MtoLStoN MtoLStoN merged commit 77f65c6 into grimme-lab:main Dec 4, 2023
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