Solve problems in arc's directed scans. #602
Conversation
Codecov Report
@@ Coverage Diff @@
## main #602 +/- ##
=======================================
Coverage 72.16% 72.16%
=======================================
Files 97 97
Lines 25564 25608 +44
Branches 5392 5408 +16
=======================================
+ Hits 18447 18479 +32
- Misses 5758 5768 +10
- Partials 1359 1361 +2
... and 2 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
| @@ -233,6 +233,8 @@ def parse_geometry(path: str) -> Optional[Dict[str, tuple]]: | |||
| content = read_yaml_file(path) | |||
| if isinstance(content, dict) and 'xyz' in content.keys(): | |||
| return content['xyz'] | |||
| if isinstance(content, dict) and 'opt_xyz' in content.keys(): | |||
There was a problem hiding this comment.
Perhaps change the tani script to store the xyz output in an xyz key instead of the opt_xyz key. We wouldn’t want additional future adapters to add their own custom keys for the geometry (unless there’s good reason to) and have to accommodate for them in this (and other?) functions
There was a problem hiding this comment.
There is an important distinction between xyz and opt_xyz, how would we differentiate them without using different keys?
I don't think it's just the torchani adapter that relates differently to these properties..
There was a problem hiding this comment.
@kfir4444, let me know when this is resolved, and let's merge this in
There was a problem hiding this comment.
@alongd I will probably add more commits to this PR before
There was a problem hiding this comment.
Was this addressed? Are we leaving it both 'xyz' and 'opt_xyz'?
kfir4444
force-pushed
the
dubg_nd_scans
branch
from
06c825b
to
7a91585
Compare
Thanks for the commants @alongd! |
kfir4444
force-pushed
the
dubg_nd_scans
branch
2 times, most recently
from
3caaa75
to
70934c5
Compare
| @@ -233,6 +233,8 @@ def parse_geometry(path: str) -> Optional[Dict[str, tuple]]: | |||
| content = read_yaml_file(path) | |||
| if isinstance(content, dict) and 'xyz' in content.keys(): | |||
| return content['xyz'] | |||
| if isinstance(content, dict) and 'opt_xyz' in content.keys(): | |||
There was a problem hiding this comment.
Was this addressed? Are we leaving it both 'xyz' and 'opt_xyz'?
kfir4444
force-pushed
the
dubg_nd_scans
branch
2 times, most recently
from
7a0ff12
to
3b13424
Compare
This would cause problems in future processing.
This is engaged if the user did not request an opt job.
Sometimes, a user may not request a certain job type, but still provide a `level` argument. This detaches the need verify if a job type is true before making the level of theory to a ``Level`` object.
A few functions returned a
np.float64, which caused errors when dumped into yaml files, and several other small corrections.