SCF: Add SCFConvergence Error #1130

obrien951 · 2018-08-08T20:52:49Z

Description

SCF: Add SCFConvergence Error such that the error will have the wavefunction as a member.

Checklist

Tests added for any new features
All or relevant fraction of full tests run

Status

Ready for review
Ready for merge

…unction fixes psi4#1062

loriab · 2018-08-08T21:49:32Z

@CDSherrill

This was the v1.2 behavior so print error msg or die, depending on settings.

Previous scfitertopy behavior was always to throw generic ConvergenceError with msg and niter.

Now, throw SCFConvergenceError with msg, niter, and wfn attached.

loriab · 2018-08-08T21:53:08Z

psi4/driver/p4util/exceptions.py

@@ -85,6 +85,7 @@ class ConvergenceError(PsiException):
    def __init__(self, eqn_description, maxit):
        msg = "Could not converge %s in %d iterations." % (eqn_description, maxit)
        PsiException.__init__(self, msg)
+        self.maxit = maxit


Maybe self.iteration or self.iter? This could be thrown under < maxit circumstances. In fact, someday there might be MaxiterExceededSCFConvergenceError, OscillationDetectedSCFConvergenceError, etc.

self.iteration would be a huge help. I would also suggest self.e_convergence and self.d_convergence would be a huge future boon so that it allows users to inspect some of the circumstances of the failure and determine if it passes a "good enough" mark.

self.maxit is the iteration number. You're agreeing to rename the exception member data?

I think e_ and d_convergence can be extracted from the wfn. You're suggesting appending current energy and current rms density to error?

Yes to renaming the member data.

Unless I am missing something it does not appear that you can extract these quantities from the Wavefunction. At the same time I would argue iter/e_conv/d_conv data makes up the current "convergence state" of the wavefunction and should be carried around together so either get all of these variables from the wavefunction or get none.

loriab · 2018-08-08T22:02:51Z

@CDSherrill

Oh, and if an error's raised but not caught, it just shows the traceback and triggers coffee:

PsiException: Could not converge SCF iterations in 4 iterations.

  Failed to converge.

Traceback (most recent call last):
  File "stage/usr/local/psi4/bin/psi4", line 269, in <module>
    exec(content)
  File "<string>", line 30, in <module>
  File "/home/psilocaluser/gits/hrw-patch/objdir/stage/usr/local/psi4/lib/psi4/driver/driver.py", line 492, in energy
    wfn = procedures['energy'][lowername](lowername, molecule=molecule, **kwargs)
  File "/home/psilocaluser/gits/hrw-patch/objdir/stage/usr/local/psi4/lib/psi4/driver/procrouting/proc.py", line 2008, in run_scf
    scf_wfn = scf_helper(name, post_scf=False, **kwargs)
  File "/home/psilocaluser/gits/hrw-patch/objdir/stage/usr/local/psi4/lib/psi4/driver/procrouting/proc.py", line 1362, in scf_helper
    e_scf = scf_wfn.compute_energy()
  File "/home/psilocaluser/gits/hrw-patch/objdir/stage/usr/local/psi4/lib/psi4/driver/procrouting/scf_proc/scf_iterator.py", line 93, in scf_compute_energy
    raise e
  File "/home/psilocaluser/gits/hrw-patch/objdir/stage/usr/local/psi4/lib/psi4/driver/procrouting/scf_proc/scf_iterator.py", line 86, in scf_compute_energy
    self.iterations()
  File "/home/psilocaluser/gits/hrw-patch/objdir/stage/usr/local/psi4/lib/psi4/driver/procrouting/scf_proc/scf_iterator.py", line 358, in scf_iterate
    raise ConvergenceError("""SCF iterations""", self.iteration_)

ConvergenceError: Could not converge SCF iterations in 4 iterations.

    Psi4 stopped on: Wednesday, 08 August 2018 05:42PM
    Psi4 wall time for execution: 0:00:01.01

*** Psi4 encountered an error. Buy a developer more coffee!
*** Resources and help at github.com/psi4/psi4.

It looks like die_if_not_converged toggled print-error-msg/die+traceback previously for SCF and DETCI, with defaulting toward print-error-msg. I think it's better to default toward die (which this PR now does) and ppl can catch the error if they want print-error-msg.

For a long time we had geometry optimizations exit smoothly even if failed for iterations exceeded. That caused some problems (I think @dsirianni hit them), so we switched toward the die+traceback. Now SCF will behave like optimizations.

CDSherrill · 2018-08-08T22:59:12Z

@loriab: This sounds good --- I think die+traceback is probably the right default if the exception isn't caught.

…

On Wed, Aug 8, 2018 at 6:02 PM Lori A. Burns ***@***.***> wrote: @CDSherrill <https://github.com/CDSherrill> Oh, and if an error's raised but not caught, it just shows the traceback and triggers coffee: PsiException: Could not converge SCF iterations in 4 iterations. Failed to converge. Traceback (most recent call last): File "stage/usr/local/psi4/bin/psi4", line 269, in <module> exec(content) File "<string>", line 30, in <module> File "/home/psilocaluser/gits/hrw-patch/objdir/stage/usr/local/psi4/lib/psi4/driver/driver.py", line 492, in energy wfn = procedures['energy'][lowername](lowername, molecule=molecule, **kwargs) File "/home/psilocaluser/gits/hrw-patch/objdir/stage/usr/local/psi4/lib/psi4/driver/procrouting/proc.py", line 2008, in run_scf scf_wfn = scf_helper(name, post_scf=False, **kwargs) File "/home/psilocaluser/gits/hrw-patch/objdir/stage/usr/local/psi4/lib/psi4/driver/procrouting/proc.py", line 1362, in scf_helper e_scf = scf_wfn.compute_energy() File "/home/psilocaluser/gits/hrw-patch/objdir/stage/usr/local/psi4/lib/psi4/driver/procrouting/scf_proc/scf_iterator.py", line 93, in scf_compute_energy raise e File "/home/psilocaluser/gits/hrw-patch/objdir/stage/usr/local/psi4/lib/psi4/driver/procrouting/scf_proc/scf_iterator.py", line 86, in scf_compute_energy self.iterations() File "/home/psilocaluser/gits/hrw-patch/objdir/stage/usr/local/psi4/lib/psi4/driver/procrouting/scf_proc/scf_iterator.py", line 358, in scf_iterate raise ConvergenceError("""SCF iterations""", self.iteration_) ConvergenceError: Could not converge SCF iterations in 4 iterations. Psi4 stopped on: Wednesday, 08 August 2018 05:42PM Psi4 wall time for execution: 0:00:01.01 *** Psi4 encountered an error. Buy a developer more coffee! *** Resources and help at github.com/psi4/psi4. It looks like die_if_not_converged <https://github.com/psi4/psi4/blob/master/psi4/src/psi4/libpsi4util/process.cc#L105-L113> toggled print-error-msg/die+traceback previously for SCF and DETCI, with defaulting toward print-error-msg. I think it's better to default toward die (which this PR now does) and ppl can catch the error if they want print-error-msg. For a long time we had geometry optimizations exit smoothly even if failed for iterations exceeded. That caused some problems (I think @dsirianni <https://github.com/dsirianni> hit them), so we switched toward the die+traceback. Now SCF will behave like optimizations. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1130 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AC9QdvbzSmwMSXIINgOIb089mSzOfCS1ks5uO2AMgaJpZM4V0pOZ> .

dgasmith · 2018-08-10T14:31:47Z

psi4/driver/p4util/exceptions.py

@@ -85,6 +85,7 @@ class ConvergenceError(PsiException):
    def __init__(self, eqn_description, maxit):
        msg = "Could not converge %s in %d iterations." % (eqn_description, maxit)
        PsiException.__init__(self, msg)
+        self.maxit = maxit


self.iteration would be a huge help. I would also suggest self.e_convergence and self.d_convergence would be a huge future boon so that it allows users to inspect some of the circumstances of the failure and determine if it passes a "good enough" mark.

dgasmith · 2018-08-10T14:33:09Z

psi4/driver/p4util/exceptions.py

+class SCFConvergenceError(ConvergenceError):
+    """Error called for problems with SCF iterations."""
+
+    def __init__(self, eqn_description, maxit, wfn):


I would try to put more important args first. So something more like:
def __init__(self, wfn, wfn_name, maxiter, e_conv, d_conv):

dgasmith · 2018-08-10T14:49:10Z

psi4/driver/p4util/exceptions.py

@@ -85,6 +85,7 @@ class ConvergenceError(PsiException):
    def __init__(self, eqn_description, maxit):
        msg = "Could not converge %s in %d iterations." % (eqn_description, maxit)
        PsiException.__init__(self, msg)
+        self.maxit = maxit


Yes to renaming the member data.

Unless I am missing something it does not appear that you can extract these quantities from the Wavefunction. At the same time I would argue iter/e_conv/d_conv data makes up the current "convergence state" of the wavefunction and should be carried around together so either get all of these variables from the wavefunction or get none.

dgasmith · 2018-08-10T14:50:22Z

psi4/driver/procrouting/scf_proc/scf_iterator.py

-            except ConvergenceError:
-                raise ConvergenceError("""SCF DF preiterations""", self.iteration_)
+            except SCFConvergenceError:
+                raise SCFConvergenceError("""SCF DF preiterations""", self.iteration_, self)


Looking through the code we probably need to call scf_finalize_energy here. There are many variables set in the scf_finalize_energy/scf_print_energies functions which we should probably change at some point (or just rename the functions).

Updated SCFConvergenceError based on github comments

loriab · 2018-08-22T20:50:03Z

ok, various issues have been addressed during the Sherrill Group programming exercise. Ready for re-review.

dgasmith

LGTM

SCF: Add SCFConvergence Error such that the error will have the wavef…

0367717

…unction fixes psi4#1062

loriab reviewed Aug 8, 2018

View reviewed changes

loriab added this to the Psi4 1.3 milestone Aug 8, 2018

loriab added enhancement python labels Aug 8, 2018

robertodr approved these changes Aug 10, 2018

View reviewed changes

dgasmith requested changes Aug 10, 2018

View reviewed changes

dgasmith reviewed Aug 10, 2018

View reviewed changes

CDSherrill and others added 2 commits August 22, 2018 15:41

Updated SCFConvergenceError based on github comments

7c5f75a

Merge pull request #1 from CDSherrill/SCF_exc_wfn

af5f59d

Updated SCFConvergenceError based on github comments

dgasmith approved these changes Sep 6, 2018

View reviewed changes

loriab approved these changes Sep 6, 2018

View reviewed changes

loriab merged commit f7c1a59 into psi4:master Sep 6, 2018

dgasmith mentioned this pull request Dec 6, 2018

Psi4 1.3 Release Notes #1406

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SCF: Add SCFConvergence Error #1130

SCF: Add SCFConvergence Error #1130

obrien951 commented Aug 8, 2018

loriab commented Aug 8, 2018

loriab Aug 8, 2018

dgasmith Aug 10, 2018

loriab Aug 10, 2018

dgasmith Aug 10, 2018

loriab commented Aug 8, 2018

CDSherrill commented Aug 8, 2018 via email

dgasmith Aug 10, 2018

dgasmith Aug 10, 2018

dgasmith Aug 10, 2018

dgasmith Aug 10, 2018

loriab commented Aug 22, 2018

dgasmith left a comment

SCF: Add SCFConvergence Error #1130

SCF: Add SCFConvergence Error #1130

Conversation

obrien951 commented Aug 8, 2018

Description

Checklist

Status

loriab commented Aug 8, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

loriab commented Aug 8, 2018

CDSherrill commented Aug 8, 2018 via email

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

loriab commented Aug 22, 2018

dgasmith left a comment

Choose a reason for hiding this comment