Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ExtremelyAsyn -restart tested #88

Closed
wants to merge 8 commits into from

Conversation

joannaqw
Copy link

  1. Made a new class extremeAsynLASSCF, which is a subclass of LASSCFNoSymm
  2. within the new class, modified rdm_cycle and ci_cycle to read RDMs, write Hamiltonians and break
  3. added reading and writing orbitals (mo_coeff) within the kernel function

@MatthewRHermes
Copy link
Owner

MatthewRHermes commented Apr 29, 2024

Could you resolve conflicts please? (Pull master to your local branch, resolve the conflict as you see fit, commit, and push to your remote branch and it will update here.) The reason that's important is that it will enable me to run the Github Actions unittest workflow to see if this breaks anything.

@@ -117,48 +118,60 @@ def get_init_guess_rdm (las, mo_coeff=None, h2eff_sub=None):
def _ddm (dm1frs0, dm1frs1):
return np.concatenate ([(d1 - d0).ravel () for d1, d0 in zip (dm1frs0, dm1frs1)])


Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the purpose of the changes to this function? It seems you replace it in your subclass anyway.

converged = True
break
log.info ("LASSCF rdm-jk {}".format (('not converged','converged')[int(converged)]))
return e_cas, casdm1frs, casdm2fr, converged

def kernel (las, mo_coeff=None, casdm1frs=None, casdm2fr=None, conv_tol_grad=1e-4, verbose=lib.logger.NOTE):
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Editing this function changes the behavior of the existing lasscf_rdm implementation as well as your new implementation. Can you instead write a separate kernel function for your subclass, if you must change something in kernel?

Copy link
Owner

@MatthewRHermes MatthewRHermes May 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or, alternatively, you could instead write "hook" functions that do nothing for the lasscf_rdm.LASSCFNoSymm class but which do the I/O for the lasscf_rdm.extremeAsynLASSCF class.

file_path = os.path.join(directory_path, 'guessOrb.h5')
with h5py.File(file_path, 'r') as f:
mo_coeff = f['guessOrb'][:]
print("read the previous orbital")
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should not use "print". Use the logger commands instead.

for it in range (las.max_cycle_macro):
e_cas, casdm1frs, casdm2fr, rdmjk_conv = rdm_cycle (las, mo_coeff, casdm1frs,
for it in range (las.max_cycle_macro): ###las.rdm_cycle
e_cas, casdm1frs, casdm2fr, rdmjk_conv = las.rdm_cycle (mo_coeff, casdm1frs,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is kind of an example of a "hook" that my comment referred to above. las.rdm_cycle does one thing for lasscf_rdm.LASSCFNoSymm, and another thing for lasscf_rdm.extremeAsymLASSCF.

@@ -503,6 +525,116 @@ def kernel(self, mo_coeff=None, casdm1frs=None, casdm2fr=None, conv_tol_grad=Non

return self.e_tot, self.e_cas, self.casdm1frs, self.casdm2fr, self.mo_coeff, self.mo_energy, h2eff_sub, veff

class extremeAsynLASSCF (LASSCFNoSymm):
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At a certain point, if this gets too big, it would make sense to put it in a new file.

@@ -4,7 +4,7 @@
from scipy.sparse import linalg as sparse_linalg
from scipy import linalg
import numpy as np

import h5py
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this necessary?

@MatthewRHermes
Copy link
Owner

MatthewRHermes commented May 6, 2024

IDK if we're moving forward with this at this point but it might be a good exercise to respond to my comments and see if we can reach a good point with this PR, if we have time.

@joannaqw joannaqw closed this Jul 9, 2024
@joannaqw
Copy link
Author

joannaqw commented Jul 9, 2024

Going to keep this branch separated (on my fork) while Mario and I continue working on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants