Skip to content

Initialize UCJ from CISD#595

Merged
kevinsung merged 6 commits intoqiskit-community:mainfrom
davidomanovic:feature/ucj_from_cisd
Mar 31, 2026
Merged

Initialize UCJ from CISD#595
kevinsung merged 6 commits intoqiskit-community:mainfrom
davidomanovic:feature/ucj_from_cisd

Conversation

@davidomanovic
Copy link
Copy Markdown
Contributor

@davidomanovic davidomanovic commented Mar 16, 2026

Initialization of UCJ from CISD as described in https://arxiv.org/pdf/2511.22476

Added as a method from.cisd() in UCJOpSpinBalanced, and any necessary tests. Closes issue #592.

image

def from_cisd(
cisd,
*,
civec: np.ndarray | None = None,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Instead of accepting cisd, only accept civec, and rename it to cisd_vec. Also, rename the method to from_cisd_vec.

"or use a converged PySCF CISD object with a `ci` attribute."
)

c0, c1, c2 = cisd.cisdvec_to_amplitudes(civec, copy=False)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Use pyscf.ci.cisd.cisdvec_to_amplitudes instead, since you won't have the cisd object anymore.

"UCJOpSpinBalanced.from_cisd requires amplitudes from a restricted "
"closed-shell CISD calculation."
)
if np.isclose(c0, 0.0):
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Add a keyword-only argument c0_tol: float = 1e-8 to use for the absolute tolerance here. Also, this is a nit but prefer math.isclose to np.isclose when comparing numbers because it avoids numpy function dispatch overhead.

Comment thread python/ffsim/variational/ucj_spin_balanced.py Outdated
Comment thread python/ffsim/variational/ucj_spin_balanced.py Outdated
]
| None = None,
tol: float = 1e-8,
c0_tol: float = 1e-8,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Let's put c0_tol after nocc and before n_reps.

:meth:`from_t_amplitudes`.

Args:
civec: CISD coefficient vector.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Need to update this to cisd_vec and also add document norb, nocc, and c0_tol.

Comment on lines +614 to +616
if cisd_vec is None:
raise ValueError("CISD coefficient vector `cisd_vec` cannot be None.")

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This check is unnecessary.

cisd_vec, nmo, nocc, copy=False
)

if math.isclose(c0, 0.0, rel_tol=0.0, abs_tol=c0_tol):
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Since we're comparing to zero, it's actually unnecessary to specify rel_tol.

Comment on lines +623 to +624
f"CISD reference coefficient c0={c0} is too close to zero "
f"(abs_tol={c0_tol})."
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
f"CISD reference coefficient c0={c0} is too close to zero "
f"(abs_tol={c0_tol})."
f"CISD reference coefficient c0={c0} is smaller than the specified threshold, c0_tol={c0_tol}."

t2 = c2 / c0 - 0.5 * np.einsum("ia,jb->ijab", t1, t1)

operator = ffsim.UCJOpSpinBalanced.from_cisd_vec(
cisd.ci, nmo=cisd.nmo, nocc=cisd.nocc
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

You should also test that cisd.ci was not modified, since you chose to pass copy=False in cisdvec_to_amplitudes.

Copy link
Copy Markdown
Collaborator

@kevinsung kevinsung left a comment

Choose a reason for hiding this comment

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

Thank you! Just a few more minor changes.

Comment thread python/ffsim/variational/ucj_spin_balanced.py Outdated
Comment thread python/ffsim/variational/ucj_spin_balanced.py Outdated
Comment thread python/ffsim/variational/ucj_spin_balanced.py Outdated
Comment thread python/ffsim/variational/ucj_spin_balanced.py Outdated
Comment thread python/ffsim/variational/ucj_spin_balanced.py Outdated
@davidomanovic davidomanovic force-pushed the feature/ucj_from_cisd branch 2 times, most recently from c4d1e16 to 1e3131f Compare March 19, 2026 10:13
@davidomanovic
Copy link
Copy Markdown
Contributor Author

The current CI failures appear to be unrelated to this change.

The failing jobs are invoking tox with environment names like py3.14, but tox.ini defines the test environments as py310, py311, py312, py313, and py314:

env_list =
py{310,311,312,313,314}

For example, one failing job reports:
tox run -e py3.14
and tox returns:
provided environments not found in configuration file: py3.14 - did you mean py314?

So at least these failures are due to a workflow/tox environment naming mismatch @kevinsung

@davidomanovic davidomanovic requested a review from kevinsung March 19, 2026 10:19
@kevinsung
Copy link
Copy Markdown
Collaborator

The current CI failures appear to be unrelated to this change.

The failing jobs are invoking tox with environment names like py3.14, but tox.ini defines the test environments as py310, py311, py312, py313, and py314:

env_list = py{310,311,312,313,314}

For example, one failing job reports: tox run -e py3.14 and tox returns: provided environments not found in configuration file: py3.14 - did you mean py314?

So at least these failures are due to a workflow/tox environment naming mismatch @kevinsung

CI was broken because of a change in tox 4.50.1. I've fixed this in #603 , please merge main.

@davidomanovic
Copy link
Copy Markdown
Contributor Author

The current CI failures appear to be unrelated to this change.

The failing jobs are invoking tox with environment names like py3.14, but tox.ini defines the test environments as py310, py311, py312, py313, and py314:

env_list = py{310,311,312,313,314}

For example, one failing job reports: tox run -e py3.14 and tox returns: provided environments not found in configuration file: py3.14 - did you mean py314?

So at least these failures are due to a workflow/tox environment naming mismatch @kevinsung

CI was broken because of a change in tox 4.50.1. I've fixed this in #603 , please merge main.

Hmmm still something wrong with tox.

@kevinsung
Copy link
Copy Markdown
Collaborator

Just need to fix the sphinx warnings now. Unfortunately, I've never figured out a good way to debug those, besides tracking down the offending line through binary search.

*,
norb: int,
nocc: int,
c0_threshold: float = 1e-8,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
c0_threshold: float = 1e-8,
c0_threshold: float = 1e-12,

Let's make this smaller actually.

Comment on lines +572 to +580
cisd_vec: The CISD vector. This is a one-dimensional array storing the
reference coefficient :math:`c_0` in the first entry, followed by the
singles and then doubles coefficients.
norb: The number of spatial orbitals.
nocc: The number of occupied orbitals.
c0_threshold: Absolute value threshold for the reference coefficient.
An error is raised if the absolute value of the reference coefficient is
smaller than this threshold.
n_reps: The number of ansatz repetitions.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The indentation here doesn't match the other arguments, where lines after the first one are indented. Is this related to the Sphinx issues you were having?

@kevinsung
Copy link
Copy Markdown
Collaborator

Hmm the CI failure seems unrelated, I will look into it...

Copy link
Copy Markdown
Collaborator

@kevinsung kevinsung left a comment

Choose a reason for hiding this comment

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

Thank you!

@kevinsung kevinsung merged commit 250d579 into qiskit-community:main Mar 31, 2026
27 of 28 checks passed
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