Closed soranjh closed 4 years ago
Merging #434 into master will increase coverage by
0.09%
. The diff coverage is99.36%
.
@@ Coverage Diff @@
## master #434 +/- ##
==========================================
+ Coverage 97.84% 97.94% +0.09%
==========================================
Files 58 70 +12
Lines 6784 7107 +323
==========================================
+ Hits 6638 6961 +323
Misses 146 146
Impacted Files | Coverage Δ | |
---|---|---|
strawberryfields/apps/plot.py | 100.00% <ø> (ø) |
|
strawberryfields/apps/train/embed.py | 100.00% <ø> (ø) |
|
...awberryfields/backends/gaussianbackend/__init__.py | 100.00% <ø> (ø) |
|
strawberryfields/backends/gaussianbackend/ops.py | 100.00% <ø> (+2.70%) |
:arrow_up: |
strawberryfields/circuitspecs/circuit_specs.py | 100.00% <ø> (ø) |
|
strawberryfields/circuitspecs/gaussian_unitary.py | 100.00% <ø> (ø) |
|
strawberryfields/circuitspecs/tensorflow.py | 100.00% <ø> (ø) |
|
strawberryfields/backends/fockbackend/ops.py | 94.04% <94.11%> (+2.04%) |
:arrow_up: |
strawberryfields/ops.py | 98.84% <96.29%> (-0.19%) |
:arrow_down: |
strawberryfields/backends/tfbackend/ops.py | 96.69% <97.88%> (-0.17%) |
:arrow_down: |
... and 51 more |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update d77ed6a...c40de43. Read the comment docs.
Thank you @soranjh. I have left some comments.
Question: is there a utility function to extract the input parameters from the output of the frequency calculations for the initial and final states?.
We are thinking of adding it.
Thanks Tom!
- Some of the equations in the docstring don't seem to be matching what is going on in the function, in particular
m
cropping up in unexpected places in the function. It would be good if both could match to make future maintainability easier.
The equations in the code and in the docstring match now. (they were looking different because quantum chemistry packages typically print normal modes in mass-unweighted format and I thought we can help the users by just regenerating mass-weighted ones in the code. But it can be confusing for people that are not familiar with outputs of quantum chemistry packages so I reorganised things and mentioned the format of the normal modes clearly in the docstring Args.)
- The input and output dimensions of the function surprised me, I'm curious why we expect 1 dimensional output?
Please note that the vibrational normal modes for a molecule containing N atoms are (3N) x (3N - 6) matrices for nonlinear molecules and (3N) x (3N - 5) matrices for linear molecules. 3N - 6 and 3N - 5 represent the number of vibrational modes. Note that 6 and 5 here refer to the sum of translational and rotational degrees of freedom that should be subtracted from the total degrees of freedom (3N) to obtain the number of vibrational degrees of freedom (which is equal to the number for vibrational modes).
So for a diatomic molecule, which is necessarily linear, we have only 1 (= 3 * 2 - 5) vibrational normal mode that is defined by 6 Cartesian displacement components (3 for each atom). In this case the Duschinsky rotation matrix is simply a 1 x 1 matrix and the displacement vector also has one component. But for a nonlinear molecule with 4 atoms provided in the test_utils_qchem
, we have 6 vibrational modes and the Duschinsky rotation matrix is a 6 x 6 matrix and the displacement vector also has 6 components. I could use a larger example in the "Example usage" section but as you can see, the amount of data is huge and it does not render nicely.
I have added a paragraph to the docstring to explain these details brieifly. Please also note that we are thinking of adding a function that reads the output of quantum chemistry data and extracts the normal modes. We can explain the fundamentals a bit more in that function or maybe in a tutorial.
This PR adds the function
duschinsky
tosf.apps.qchem.utils
for computing Duschinsky rotation matrix and displacement vector.