pp.x: fix gaussian cube file parsing. #535
Conversation
|
@yakutovicha Seems like one parser test is failing after I updated the branch to include the latest changes to support multiple file parsing for |
yakutovicha
force-pushed
the
fix_gaussian_parsing
branch
from
1a3253e
to
58d0fc2
Compare
Fix gaussian cube file parsing and add structure parsing to it.
yakutovicha
force-pushed
the
fix_gaussian_parsing
branch
from
58d0fc2
to
8434525
Compare
The cube files were wrong. The number of data points per dimension is set to 1, but three data points instead of one were provided in the data array. I modified the cube files. |
There was a problem hiding this comment.
Thanks @yakutovicha . Changes to parsing of volumetric data is fine, but not sure why you added the other changes. I would remove them because they are beyond the scope of this PR and they are just adding redundant information.
| @@ -217,7 +217,7 @@ def detect_important_message(logs, line): | |||
| split_line = line.split('=') | |||
| if 'negative/imaginary' in line: # QE6.1 | |||
| output_dict['negative_core_charge'] = float(split_line[-1].split()[0]) | |||
| output_dict['imaginary_core_charge'] = float(split_line.split()[-1]) | |||
| output_dict['imaginary_core_charge'] = float(split_line[-1].split()[-1]) | |||
There was a problem hiding this comment.
This is apparently not being tested, correct? I guess that is because the output files we have are not from QE 6.1. However, why is there no imaginary_core_charge for the QE 6.4 branch?
There was a problem hiding this comment.
This is apparently not being tested, correct?
Looks like it wasn't.
However, why is there no imaginary_core_charge for the QE 6.4 branch?
I don't know why, I can just say that there are no imaginary charges for QE 6.4, indeed:
I should also mention, that in QE 6.5 this line is not present at all, while for 6.3 it is present, and looks as follows:
Check: negative/imaginary core charge= -0.000001 0.000000
Thanks for the review @sphuber. In the reply to your comments I provided the reasoning why, I believe, parsing of the structure should be introduced. It is fine for me to add it in a separate PR if you agree with this change. |
|
not sure why some tests are failing now, but I believe it is unrelated to the changes in this PR. |
|
Yeah, this also happened on Edit: actually, I am not sure if I can workaround this, because the problem is with the runners running Python 3.7. In |
yakutovicha
force-pushed
the
fix_gaussian_parsing
branch
from
f89ecb6
to
e0119a9
Compare
|
Thanks a lot @yakutovicha |
The current implementation assumes that the number length is 13 digits, which is not
transferrable. In addition, it uses python loops, which are slow.
The PR replaces loops used to reshape the final array by the
reshapemethod anduses
line.split()to split the data values in every line.