Refactor optimization #83
Conversation
erikkjellgren
commented
Dec 2, 2024
erikkjellgren
commented
- All wave functions now have two different optimization calls, run_wf_optimization_1step and run_wf_optimization_1step
- Wave functions no longer take in num_spin_orbs as an argument
- The circuit based wave function has changed name from WaveFunction to WaveFunctionCircuit
- Changed c_trans to c_mo to make it more clear it is the mo coefficients.
- All wave functions now have self.thetas for the active-space parameterization, i.e. self.ansatz_parameters have been removed from the circuit based wave function.
…actor_optimization
…actor_optimization
…actor_optimization
…actor_optimization
…actor_optimization
| H = hamiltonian_0i_0a(self.h_mo, self.g_mo, self.num_inactive_orbs, self.num_active_orbs) | ||
| H = H.get_folded_operator(self.num_inactive_orbs, self.num_active_orbs, self.num_virtual_orbs) | ||
| return self.QI.quantum_expectation_value(H) | ||
| rdms = ReducedDenstiyMatrix( |
There was a problem hiding this comment.
Why is the energy only calcuated via RDM if theta_optimization = False?
There was a problem hiding this comment.
Because RDM construction is the same price or more expensive than calculating the Hamiltonian (some of the RDM terms might not be needed for the energy because the integrals are zero).
But if theta_optimization is false, then the RDM's calculated to get the energy will also be used to get the gradients for the orbital optimization, hence it is worth it then.
There was a problem hiding this comment.
Ok. makes sense. How about a one-liner comment explaining this
| num_kappa = 0 | ||
| gradient = np.zeros(len(parameters)) | ||
| num_kappa = 0 | ||
| if kappa_optimization: |
There was a problem hiding this comment.
WHy is there a move in cep of kappa in the gradient? Is this not done in the energy?
There was a problem hiding this comment.
Because I do not assume any specific implementation details of the underlying optimizers.
In principle a gradient descent does not need to evaluate the energy but could just call the gradient many times.
In this case the current-expansion-point also need to be moved.
There was a problem hiding this comment.
Ok. And otherwise it would just mean that kappa = kappa_old and the expansion point is not actually moved when calling _move_cep?
Co-authored-by: Karl Michael Ziems <39458020+ziechys@users.noreply.github.com>
Co-authored-by: Karl Michael Ziems <39458020+ziechys@users.noreply.github.com>
Co-authored-by: Karl Michael Ziems <39458020+ziechys@users.noreply.github.com>
| @@ -278,6 +256,15 @@ def ansatz_parameters(self, parameters: list[float]) -> None: | |||
| self._energy_elec = None | |||
| self.QI.parameters = parameters | |||
|
|
|||
| def _move_cep(self) -> None: | |||
| """Move current expansion point.""" | |||
There was a problem hiding this comment.
This is quite convoluted logic, I think. Let me see if I understand this correctly:
- You set integrals to none, which means next time the integrals are called, they will get updated with the current _c_mo
- You call c_mo because in that getter the new kappa is applied to _c_mo.
- You set _c_mo to the value you got above by calling its getter :D
If I am correct, this seems quite complicated? Why not just a function that updates integrals and coefficients when there is a new kappa?
Or if there is a reason, this is fine, but we need some comments here or no one will ever again understand the logic here :D
| kappa_mat[p, q] = kappa_val | ||
| kappa_mat[q, p] = -kappa_val | ||
| return np.matmul(self._c_orthonormal, scipy.linalg.expm(-kappa_mat)) | ||
| if np.max(np.abs(np.array(self.kappa) - np.array(self._kappa_old))) > 0.0: |
There was a problem hiding this comment.
A comment to why we build the kappa matrix as difference values maybe? (sth about expansion point changes)
#readibility
| @@ -26,16 +26,16 @@ | |||
| get_indexing, | |||
| ) | |||
| from slowquant.unitary_coupled_cluster.operators import Epq, hamiltonian_0i_0a | |||
| from slowquant.unitary_coupled_cluster.optimizers import Optimizers | |||
| from slowquant.unitary_coupled_cluster.util import UccStructure | |||
|
|
|||
|
|
|||
| class WaveFunctionUCC: | |||
There was a problem hiding this comment.
Harmonize (new) comments to be equal in all wave function classes
| @@ -27,16 +27,16 @@ | |||
| propagate_unitary, | |||
| ) | |||
| from slowquant.unitary_coupled_cluster.operators import Epq, hamiltonian_0i_0a | |||
| from slowquant.unitary_coupled_cluster.optimizers import Optimizers | |||
| from slowquant.unitary_coupled_cluster.util import UpsStructure | |||
|
|
|||
|
|
|||
| class WaveFunctionUPS: | |||
There was a problem hiding this comment.
Harmonize (new) comments to be equal in all wave function classes
| @@ -30,16 +30,16 @@ | |||
| hamiltonian_0i_0a, | |||
| one_elec_op_0i_0a, | |||
| ) | |||
| from slowquant.unitary_coupled_cluster.optimizers import Optimizers | |||
| from slowquant.unitary_coupled_cluster.util import UpsStructure | |||
|
|
|||
|
|
|||
| class WaveFunctionSAUPS: | |||
There was a problem hiding this comment.
Harmonize (new) comments to be equal in all wave function classes
