Merge functionalities into old cc_<geminal>.py files from their _NEW counterpart by q-pratz-chem · Pull Request #201 · mqcomplab/Fanpy

q-pratz-chem · 2026-05-16T17:50:13Z

Consolidating the legacy CC/geminal wavefunction implementations with their corresponding _NEW variants.

The primary goal of this PR is to:

Copy missing functionality from _NEW implementations into the canonical/legacy implementations,
Verify that the merged implementation preserves existing functionality,
Resolve inconsistencies between old and _NEW implementations where needed,
Improve documentation and mathematical consistency,
Eventually, remove the duplicated _NEW files once consolidation is complete.

This PR focuses mainly on review, consolidation, documentation cleanup, and targeted testing additions where functionality differences existed.

Main Changes in Files

`pccd_ap1rog.py`

Merged functionality from _NEW into the canonical implementation, including:

s_type functionality and overlap filtering logic,
_olp_double_derivative,
associated documentation updates,
and new tests for the merged functionality.

The seniority-0 reference-wavefunction check from _NEW was not adopted because it incorrectly used slicing over :nspatial instead of nelec // 2.

Important Note

The _NEW implementation of _olp_deriv and _olp_double_derivative uses:

occ_indices(sd)

rather than:

occ_indices(sd2)

This behavior was preserved for now because the implementation was used in calculations associated with the paper results. Further investigation is still needed to confirm whether this behavior is theoretically correct.

`ap1rog_spin.py`

`ap1rog_generalized.py`

`apg1ro_d.py`

`apg1ro_sd.py`

`apset1rog_d.py`

`apset1rog_sd.py`

No functional differences were found between the canonical and _NEW implementations (aside from parent import filenames, which were left unchanged since only old files are modified to accommodate the functionalities from their _NEW variants).

For these files:

Wavefunction equations and docstrings were corrected and modernized,
Excitation-operator structure and overlap-generation workflow were clarified,
Mathematical notation was updated to better reflect the actual implementation.

No new tests were added for these files because no functionality changes were introduced during the merge review. Additional tests can be included to improve test coverage through a separate branch.

Additional Notes

A major focus of this PR was improving consistency between:

the mathematical ansatz described in the docstrings,
the excitation operators actually generated in code,
and the overlap/filtering workflow implemented in the CC infrastructure.

Older layered operator expressions- a series of parentheses for doubles and single operators- could lead to confusion on the ordering of operators. So, those were replaced with excitation-pool formulations that more accurately reflect the implementation behavior.

Copy class docstring additions from pccd_ap1rog_NEW: - document s_type attribute - document assign_s_type method - document _olp_double_derivative method Update PCCD.init: - add s_type keyword argument - add self.assign_s_type(s_type) Add assign_s_type method from pccd_ap1rog_NEW Add validation of allowed s_type values: - free - sen-o - sen-v - sen-ov (validation logic was missing in pccd_ap1rog_NEW) Merge sen-type overlap filtering logic from pccd_ap1rog_NEW into _olp Correct occupancy determinant used during excitation filtering: - replace sd with sd2 inside temp_olp inside `def _olp` - sd2 is the determinant being excited to generate sd1 - consistent with excitation logic and `pccd_ap1rog_NEW.py` implementation Change sd2 to sd in `def temp_olp` inside `def _olp_deriv` - consistent with `pccd_ap1rog_NEW.py` implementation Copied `_olp_double_deriv` function from `pccd_ap1rog_NEW.py` While merging code from pccd_ap1rog_NEW, retain the original `occ_indices(sd)` implementation in `_olp_deriv` and `_olp_double_derivative` (rather than changing to `sd2`) - add notes indicating that the correct determinant to use for occupancy filtering requires further investigation Preserve correct seniority-0 reference wavefunction validation: - **keep original slicing using nelec // 2** from `pccd_ap1rog.py` - do NOT adopt incorrect nspatial slicing from `pccd_ap1rog_NEW.py` Add implementation/documentation notes: - explain that pure PCCD generates only pair-double excitation operators - explain that s_type logic becomes active only when singles exist in exops - clarify relationship between exop structure and seniority filtering Remove stale commented-out debug print statements **Tests added:** test_assign_refwfn_sen_0_check test_assign_s_type test_assign_s_type_invalid test_init_s_type test_s_type_effect_on_pccd_overlap test_olp_double_derivative_shape test_olp_double_derivative_symmetric test_olp_double_derivative_zero_diagonal test_olp_double_derivative_finite_difference Add finite-difference validation of overlap Hessian Add Hessian symmetry test Add zero-diagonal structural Hessian test

Compare ap1rog_spin_NEW.py against the canonical ap1rog_spin.py implementation and confirm that no functional code differences were present requiring merge changes Add documentation and developer notes clarifying: - excitation-pool formulation of AP1roGSDSpin - role of singles, pair doubles, and seniority-filtered operators - interpretation of sen-o, sen-v, sen-ov, and free variants - dynamic enforcement of seniority filtering during overlap evaluation - assign_exops constructing only a static excitation operator pool - generation of compatible excitation combinations during overlap evaluation Also correct AP1roG typo in module docstring and replace outdated layered ansatz expression with implementation-consistent notation.

…entation Compare ap1rogsd_generalized_NEW.py against the canonical ap1rogsd_generalized.py implementation and confirm that no functional code differences required merging. Refine and expand documentation to better reflect the implemented excitation structure and overlap-generation workflow. Documentation updates include: - correct and modernize the AP1roGSDGeneralized ansatz expression - replace outdated layered excitation notation with excitation-pool formulation - document generalized single excitations and paired double excitations - clarify that generalized singles allow broken spin symmetry and spin-flip excitations - document sen-o, sen-v, sen-ov, and free seniority-filtering interpretations - clarify that seniority filtering is applied dynamically during overlap evaluation - add notes describing the static excitation-operator pool construction in assign_exops - clarify that overlap routines later generate compatible excitation combinations from the excitation pool Also correct minor AP1roG naming inconsistencies in docstrings.

Compare apg1ro_d_NEW.py against the canonical apg1ro_d.py implementation and confirm that no functional code differences required merging beyond import consistency. Refine and expand documentation to better reflect the implemented excitation structure and overlap-generation workflow. Documentation updates include: - modernize the APG1roD ansatz expression using excitation-pool notation - clarify that the excitation operators are generalized doubles of the form tau_{i ibar}^{pq} - document that occupied annihilators are restricted to paired occupied spin complements - document that virtual creation indices are unrestricted generalized spin orbitals - clarify distinction between pair-preserving and pair-breaking virtual excitations - document possible spin-symmetry breaking and spin contamination - clarify that inherited s_type filtering remains inactive because only double excitations are present - add notes describing static excitation-pool construction and later overlap-generation workflow Also improve terminology and mathematical consistency in docstrings.

Compare apg1ro_sd_NEW.py against the canonical apg1ro_sd.py implementation and confirm that no functional code differences required merging beyond import consistency. Refine and expand documentation to better reflect the implemented excitation structure and overlap-generation workflow. Documentation updates include: - modernize the APG1roSD ansatz expression using excitation-pool notation - replace outdated layered excitation notation with implementation-consistent operator-pool formulation - document generalized single excitations and generalized pair-annihilating double excitations - clarify that occupied annihilators in doubles remain restricted to paired occupied spin complements - clarify that virtual creation indices in doubles are unrestricted generalized spin orbitals - document distinction between pair-preserving and pair-breaking generalized doubles - clarify broken spin symmetry, spin-flip excitations, and possible spin contamination - document sen-o, sen-v, sen-ov, and free seniority-filtering interpretations for generalized singles - clarify that seniority filtering is applied dynamically during overlap evaluation - add notes describing static excitation-pool construction and later overlap-generation workflow Also improve terminology and mathematical consistency throughout the docstrings.

Compare apset1rog_d_NEW.py against the canonical apset1rog_d.py implementation and confirm that no functional code differences required merging. Refine and expand documentation to better reflect the implemented excitation structure and overlap-generation workflow. Documentation updates include: - modernize the APset1roGD ansatz expression using excitation-pool notation - clarify that excitation operators are set-restricted doubles of the form tau_{i ibar}^{ab} - document that occupied annihilators are restricted to paired occupied spin complements - document that virtual creation indices belong to two disjoint virtual spin-orbital sets - clarify default alpha/beta virtual partitioning and spin-projection preservation - document support for user-defined disjoint virtual excitation sets - clarify distinction between APset1roGD and fully generalized APG1roD excitations - clarify that inherited s_type filtering remains inactive because only double excitations are present - add notes describing static excitation-pool construction and later overlap-generation workflow Also improve terminology and mathematical consistency throughout the docstrings.

Compare apset1rog_sd_NEW.py against the canonical apset1rog_sd.py implementation and confirm that no functional code differences required merging. Refine and expand documentation to better reflect the implemented excitation structure and overlap-generation workflow. Documentation updates include: - modernize the APset1roGSD ansatz expression using excitation-pool notation - replace outdated layered excitation notation with implementation-consistent operator-pool formulation - document set-restricted spin-conserving singles and set-restricted doubles - clarify that occupied annihilators in doubles remain restricted to paired occupied spin complements - document that virtual creation indices belong to two disjoint virtual spin-orbital sets - clarify default alpha/beta virtual partitioning and spin-projection preservation - document support for user-defined disjoint virtual excitation sets - document sen-o, sen-v, sen-ov, and free seniority-filtering interpretations - clarify that seniority filtering is applied dynamically during overlap evaluation - add notes describing static excitation-pool construction and later overlap-generation workflow Also improve terminology and mathematical consistency throughout the docstrings.

Merge relevant functionality from pccd_ap1rog_NEW into old PCCD imple…

kzsigmond

The docstrings are on the longer side, and there is some repetition in a few places. I didn’t annotate every instance, but the main patterns I noticed are:

Repeated description of the excitation pool (appearing both before and after the equation)
Some inconsistency in orbital notation. At times general indices (e.g. pq) are used, while elsewhere the text refers to excitations from occupied to virtual orbitals. It might improve clarity to standardize notation, e.g. i, j for occupied, a, b for virtual, and p, q for general orbitals, as is commonly used in quantum chemistry.
The description of the seniority restriction could likely be shortened. For end users, it is usually sufficient to know what is being restricted; the equations here don’t add much additional usability value and may be more useful at the developer or reference level.

One possible structure for the docstrings could be:

Brief equation for the wavefunction, with concise explanation of symbols where needed (e.g. excitation pool definitions)
Key behavioral details (e.g. single excitations can be restricted via s_type, refwfn must be a seniority-0 reference). This can stay fairly high-level since the detailed parameter behavior is already documented in s_type and refwfn
Pointer to the relevant paper for users who want the full formalism

The main goal of the docstrings is to explain the API and how to use it, rather than fully reproducing the level of detail of a paper. In fact, too much detail can sometimes obscure important usage constraints, such as the requirement that refwfn has seniority 0.

As a longer-term improvement (not necessary for this release), it might also be useful to add a separate reStructuredText page for geminal wavefunctions in the documentation. That could be a better place for the detailed discussion.

kzsigmond · 2026-05-20T17:23:56Z

+    Although this class inherits the s_type infrastructure from PCCD,
+    the seniority-filtering conditions do not affect the present ansatz
+    because the excitation operator pool contains only double excitations.


Is there a warning message here if the user provides a seniority type for APG1roD?

At the moment, there is no warning message being prompted if the user provides a s_type for any doubles-only (1roD) wavefunction, although the wavefunction instantiation sets the default to sen-o.

To trigger the warning, the assign_s_type method in every 1roD wavefunction class should be overridden.

OR

There is a need for a singles_aware flag that will be set to True in each 1roD class, and assign_s_type in the base PCCD class should read the flag to trigger a warning.

For now, I have just added notes on the method itself in the PCCD class.

Fanpy/fanpy/wfn/cc/pccd_ap1rog.py

Lines 302 to 310 in 42f5975

def assign_s_type(self, s_type):

"""Assign the seniority option for single excitations.

Notes

-----

For doubles-only wavefunctions, these seniority restrictions

do not affect the overlap evaluation because the excitation operator

space contains only pair-double excitations and no singles.

The restrictions become relevant only when single excitations are

included in the excitation operator space.

kzsigmond · 2026-05-20T17:40:25Z

+    sen-o: Require breaking an occupied pair
+
+    .. math::
+
+        \tilde{\tau}_p^q = a_q^\dagger a_p \hat{n}_{\bar{p}}
+
+    sen-v: Forbid formation of virtual pairs
+
+    .. math::
+
+        \tilde{\tau}_p^q = a_q^\dagger a_p
+        \left( 1 - \hat{n}_{\bar{q}} \right)
+
+    sen-ov: Apply both restrictions
+
+    .. math::
+
+        \tilde{\tau}_p^q = a_q^\dagger a_p
+        \left(1 - \hat{n}_{\bar{q}} \right) \hat{n}_{\bar{p}}
+
+    free: No seniority restrictions
+
+    .. math::
+
+        \tilde{\tau}_p^q = a_q^\dagger a_p
+
+    These restrictions are enforced dynamically during excitation-operator
+    combination filtering during overlap evaluation and are not encoded
+    directly into the stored excitation operators.


This would be better at the parameter level description of sen_type. The equations themselves are not necessary to the end user, as the text itself explains the restriction. They would be more useful in developer notes, or comments if the pop up anywhere in the code. This applies to the other files that describe the seniority restriction as well.

kzsigmond · 2026-05-20T17:42:39Z


    Attributes
    ----------
    nelec : int


Is sen_type an attribute? If so, it should be here.

kzsigmond · 2026-05-20T17:43:54Z

+    NOTE: The excitation operator pool in this wavefunction consists of paired doubles 
+    inherited from PCCD together with spin-conserving single excitations.


You are describing the excitation operator pool twice. Could you please consolidate it?

kzsigmond · 2026-05-20T17:49:19Z

+    The excitation operator pool :math:\mathcal{E} consists of generalized
+    double excitations that
+        - annihilate paired occupied orbitals
+          :math:(i\bar{i})
+        - create electrons in arbitrary virtual spin orbitals
+          :math:(pq)
+
+    generated from the reference determinant.
+
+    Consequently, the ansatz includes both
+        - pair-preserving double excitations
+
+        .. math::
+
+        \tau_{i\bar{i}}^{a\bar{a}}
+
+        - and pair-breaking double excitations
+
+        .. math::
+
+        \tau_{i\bar{i}}^{ab}
+
+        where :math:b \neq \bar{a}.
+
+    Unlike AP1roG/PCCD, the virtual excitation space is not restricted to
+    paired spin complements. Consequently, this ansatz may break spin symmetry
+    and generate spin-contaminated determinants.


There is a lot of repeated information here slightly differently phrased/showcased.

kzsigmond · 2026-05-20T17:53:13Z

+
+        - and pair-breaking double excitations
+        .. math::
+        \tau_{i\bar{i}}^{ab}


Notation for excitation operators is not consistent. Sometimes you use r,s for virtual orbitals, and sometimes you use ab. Sticking to the following notation would be good:

occupied orbs: i, j, k...

virtual orbs: a, b, c...

general orbs: p, q, r, s

kzsigmond · 2026-05-20T18:01:55Z

+    Although this class inherits the s_type infrastructure from PCCD,
+    the seniority-filtering conditions do not affect the present ansatz
+    because the excitation operator pool contains only double excitations.


Warning if this is initialized with a non-default sen-type?

kzsigmond · 2026-05-20T18:02:33Z


 class APset1roGSD(APset1roGD):
-    r"""APset1roG wavefunction with single and double excitations.
+    r"""AAPset1roG wavefunction with set-restricted single and double excitations.


Suggested change

r"""AAPset1roG wavefunction with set-restricted single and double excitations.

r"""APset1roG wavefunction with set-restricted single and double excitations.

kzsigmond · 2026-05-20T18:08:24Z

+                                    break
+
+                        if not skip_row:
+                            selected_rows.append(row_ind)


Just to double check: we do not need here a block for the free seniority type? We have one for the others.

kzsigmond · 2026-05-20T18:15:40Z

Just a double check, since github is comparing the files in a weird way: olp_deriv did not get changed, it is just olp and olp_double_deriv, isn't it? That is why there is no test yet for the olp_deriv method.

q-pratz-chem added 8 commits May 15, 2026 12:14

Merge pull request #199 from q-pratz-chem/91-maint-new-wavefunctions

42f5975

Merge relevant functionality from pccd_ap1rog_NEW into old PCCD imple…

q-pratz-chem linked an issue May 16, 2026 that may be closed by this pull request

Maint: NEW wavefunctions #91

Open

kzsigmond self-requested a review May 20, 2026 15:33

kzsigmond requested changes May 20, 2026

View reviewed changes

kzsigmond added this to the v2.0.0 milestone May 28, 2026

	def assign_s_type(self, s_type):
	"""Assign the seniority option for single excitations.
	Notes
	-----
	For doubles-only wavefunctions, these seniority restrictions
	do not affect the overlap evaluation because the excitation operator
	space contains only pair-double excitations and no singles.
	The restrictions become relevant only when single excitations are
	included in the excitation operator space.

		NOTE: The excitation operator pool in this wavefunction consists of paired doubles
		inherited from PCCD together with spin-conserving single excitations.

	r"""AAPset1roG wavefunction with set-restricted single and double excitations.
	r"""APset1roG wavefunction with set-restricted single and double excitations.

Conversation

q-pratz-chem commented May 16, 2026

Main Changes in Files

pccd_ap1rog.py

Important Note

ap1rog_spin.py

ap1rog_generalized.py

apg1ro_d.py

apg1ro_sd.py

apset1rog_d.py

apset1rog_sd.py

Additional Notes

Uh oh!

kzsigmond left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

`pccd_ap1rog.py`

`ap1rog_spin.py`

`ap1rog_generalized.py`

`apg1ro_d.py`

`apg1ro_sd.py`

`apset1rog_d.py`

`apset1rog_sd.py`