psi4 / psi4numpy

Combining Psi4 and Numpy for education and development.
BSD 3-Clause "New" or "Revised" License
330 stars 151 forks source link

IR Intensities v2.0 #104

Closed kcpearce closed 3 years ago

kcpearce commented 3 years ago

Description

Will add reference implementation for computing IR intensities of molecules.

What are your new additions? Please provide a brief list.

Status

dgasmith commented 3 years ago

Thanks for tackling this!

kcpearce commented 3 years ago

@dgasmith @loriab I'm currently testing the frequencies against those computed by Psi4. Does the wfn variable store the IR intensities as well to compare against? If so, what are they called?

loriab commented 3 years ago

Should be in a field attached to wfn.frequency_analysis . let me know if it's missing. https://github.com/psi4/psi4/blob/master/tests/freq-isotope2/input.dat#L101-L116

kcpearce commented 3 years ago

Stupid question: wfn.frequency_analysis()['IR_Intensity'] contains

----------------------------------------
        Datum infrared intensity        
----------------------------------------
Data:     [  0.             0.             0.             0.             0.             0.             2.7682201519 163.8780634213   0.             0.             0.             0.            16.5538835573
   0.            27.5122695971   0.             0.            63.3981457698]
Units:    [km/mol]
doi:      None
Comment:  
Glossary: 
----------------------------------------

How do I access the 'Data' member?

loriab commented 3 years ago

ah yes, it's a Datum (https://github.com/MolSSI/QCElemental/blob/master/qcelemental/datum.py#L12). get it with .data, I think.

kcpearce commented 3 years ago

Ready for review!

kcpearce commented 3 years ago

Looks like I'm failing quite a few tests in Travis py37. Any ideas as to why?

loriab commented 3 years ago

ah, that can happen when run infrequently.

dgasmith commented 3 years ago

Looks like on Python 3.7 Psi4 1.4a3.dev51+83a8459 is failing, but 1.3.2+ecbda83 passes fine.

----------------------------- Captured stderr call -----------------------------
1388Traceback (most recent call last):
1389  File "/home/travis/build/psi4/psi4numpy/Coupled-Electron-Pair-Approximation/DF-LCCD.py", line 39, in <module>
1390    R, F = integrals_DF(mol)
1391  File "/home/travis/build/psi4/psi4numpy/Coupled-Electron-Pair-Approximation/integrals.py", line 80, in integrals_DF
1392    Ppq = np.squeeze(mints.ao_eri(zero_bas, aux, basis, basis))
1393RuntimeError: 
1394Fatal Error: Bad BraKet type in Libint2TwoElectronInt
1395Error occurred in file: /scratch/psilocaluser/conda-builds/psi4-multiout_1608334598638/work/psi4/src/psi4/libmints/eri.cc on line: 198
1396The most recent 5 function calls were:
1397
1398psi::IntegralFactory::eri(int, bool, bool)
1399psi::MintsHelper::ao_eri(std::shared_ptr<psi::BasisSet>, std::shared_ptr<psi::BasisSet>, std::shared_ptr<psi::BasisSet>, std::shared_ptr<psi::BasisSet>)

A little surprising since I'm pretty sure we build DF integrals in Psi4 tests. Any ideas @andysim @loriab.

andysim commented 3 years ago

Sorry, that's a Libint2 constraint. (Ax|mn) is allowed, but not (xA|mn), so it's a trivial fix. Here x is the zero basis set, and A is the aux.

dgasmith commented 3 years ago

Thanks @andysim. @kcpearce I know its not your PR, but would you mind patching?

kcpearce commented 3 years ago

No problem. @andysim I assume the analogous (mn|Ax) is allowed, but not (mn|xA)?

andysim commented 3 years ago

Correct! I just made PR to your branch after I found two offending integral calls in the CEPA code. If there are any calls like you mentioned in your previous comment, they'll need to be patched also. Please let me know if there are any more problems - I was hoping not to break too much with the L2 upgrade, but there seem to be a few small things.

kcpearce commented 3 years ago

@andysim I think I've caught all of the offending calls in this last commit including those two in the CEPA code

andysim commented 3 years ago

Awesome - thanks so much for fixing those. It never crossed my mind to check this before

kcpearce commented 3 years ago

1g test is still failing convergence for with a 1e-10 tolerance. Should I loosen it?

  Memory set to 476.837 MiB by Python driver.
    Point group...........................................................................PASSED
    Nuclear repulsion energy..............................................................PASSED
    SCF Energy............................................................................FAILED
----------------------------- Captured stderr call -----------------------------
[NbConvertApp] Converting notebook /home/travis/build/psi4/psi4numpy/Tutorials/01_Psi4NumPy-Basics/1g_basis-sets.ipynb to script
[NbConvertApp] Writing 5720 bytes to /tmp/tmp4uug5474/1g_basis-sets.py
Traceback (most recent call last):
  File "/tmp/tmp4uug5474/1g_basis-sets.py", line 115, in <module>
    psi4.compare_values(refscf, eb, 10, "SCF Energy")                                  
  File "/home/travis/miniconda/envs/p4env/lib/python3.7/site-packages/psi4/driver/qcdb/testing.py", line 104, in _mergedapis_compare_values
    return qcel.testing.compare_values(expected, computed, **kwargs)
  File "/home/travis/miniconda/envs/p4env/lib/python3.7/site-packages/qcelemental/testing.py", line 171, in compare_values
    return return_handler(allclose, label, message, return_message, quiet)
  File "/home/travis/miniconda/envs/p4env/lib/python3.7/site-packages/psi4/driver/p4util/testing.py", line 189, in _psi4_true_raise_handler
    raise TestComparisonError(message)
psi4.driver.p4util.exceptions.TestComparisonError:  SCF Energy: computed value (-228.957630058814) does not match (-228.957630059008) to atol=1e-10 by difference (0.000000000194).
andysim commented 3 years ago

Yeah, that's being tested a bit too tight. You can set the integral tolerance lower, but I think just loosening the check makes more sense. The answer is nondeterministic to some degree when running in parallel anyway, so super tight checks don't really make sense

kcpearce commented 3 years ago

Lowered the tolerance to 7, and looks like we're passing Travis. Finally ready for merge!

dgasmith commented 3 years ago

Nice!