[new opmath 4] Update tests to pass under new opmath #5335
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Qottmann
changed the title
|
On this branch / PR the Hamiltonian tests pass now. We should now be able to see what breaks and define and distribute tasks to update tests based on that. Note however that there are some false positives as some tests actively enable and disable new opmath, which under the new default just globally disable new opmath in the CI runner of that test afterwards. So to get a clearer picture we need to remove all those first. Theoretically, there should be no need to update the codebase, solely the tests. However there may be some unknowns. |
Qottmann
changed the title
…nnylane into ham-lc-switch
**Context:** Updates the qchem generators to return as either a `qml.ops.Hamiltonian` or `qml.ops.LinearCombination` (depending on whether op_math is enabled) as part of standardizing generators and keeping them consistent with the `op_math` setting. **Description of the Change:** These ones already returned a new opmath operator (SProd) if `active_new_opmath` and a legacy operator (Hamiltonian) otherwise, because they are created using dunder methods rather than explicit constructors. This change makes it so they will return `LinearCombination` instead, because this is more analogous to `Hamiltonian`, and some of the generator behaviour depends on that. Added tests for both old and new opmath checking the generator return type, and tagged the parameter frequency tests (functionality dependent on `op.generator()`) to run with both. **Benefits:** - The qchem op generators are consistently either "`qml.Hamiltonian`" (whatever that may mean in the current context) or `SparseHamiltonian`. - We can remove one of our `xfail` marks in the optimize folder that was a result of the discrepancy here - We can revert a test change that we missed previously that represented a real change in functionality and *should* have been an `xfail` --------- Co-authored-by: Mudit Pandey <mudit.pandey@xanadu.ai> Co-authored-by: qottmann <korbinian.kottmann@gmail.com> Co-authored-by: Alex Preciado <alex.preciado@xanadu.ai> Co-authored-by: Pietropaolo Frisoni <pietropaolo.frisoni@xanadu.ai>
**Context:** `TestHamiltonianSamples` fails in ham-tests **Description of the Change:** Update `qml.Hamiltonian` to `qml.ops.Hamiltonian` in `isinstance` checks such that it always refer to the old `Hamiltonian` [sc-59163]
After looking at the tests for a bit and realizing what was going on, I would recommend these generators return new_opmath operators regardless of whether new_opmath is installed, because the new_opmath was explicitly used to add functionality that isn't available with `qml.Hamiltonian`: - the "Identity with no wires" functionality only works with new opmath, and is needed for `ControlledGlobalPhase`. Changing `Identity` and `Controlled` is just undoing #5194. - a Hamiltonian is not suitable for the qutrit operators, because it can't return a matrix for a qutrit system, breaking parameter frequencies I've added a comment on each of the relevant generator functions, since this will differ from the standard format, and that's all. --------- Co-authored-by: Mudit Pandey <mudit.pandey@xanadu.ai> Co-authored-by: qottmann <korbinian.kottmann@gmail.com> Co-authored-by: Alex Preciado <alex.preciado@xanadu.ai> Co-authored-by: Pietropaolo Frisoni <pietropaolo.frisoni@xanadu.ai>
**Context:** After enabling the new operator arithmetic by default, we want the generators in the source code to return a `LinearCombination` instance or a `Hamiltonian` instance wherever possible. **Description of the Change:** The generators touched in this PR are modified so that they return `qml.Hamiltonian` instead of `Sum`, `Prod`, or `Sprod` instances. When opmath is enabled, `qml.Hamiltonian` points to `pennylane.ops.op_math.linear_combination.LinearCombination`, and when it is disabled, it points to `pennylane.ops.qubit.hamiltonian.Hamiltonian`. This ensures that the appropriate instance is used consistently. Note that the generators unchanged in this PR are modified (wherever possible) in #5410 , #5411 , #5412 (including the changelog entry). **Benefits:** A more coherent choice depending on whether opmath is enabled or disabled. **Possible Drawbacks:** None that I can think of, except that old opmath would be deprecated in the future. Therefore, some precautions that have been taken here (to ensure that tests associated with generators work even with opmath disabled) might become useless. **Related GitHub Issues:** None. [sc-57982] --------- Co-authored-by: Mudit Pandey <mudit.pandey@xanadu.ai> Co-authored-by: lillian542 <Lillian.frederiksen@xanadu.ai> Co-authored-by: qottmann <korbinian.kottmann@gmail.com> Co-authored-by: Alex Preciado <alex.preciado@xanadu.ai>
…ithmetic (#5421) **Context:** We are generally treating "Hamiltonian" as meaning alternately `qml.ops.Hamiltonian` or `qml.ops.LinearCombination` depending on whether or not `new_opmath` is enabled. The current implementation of `qml.generator(op, format="hamiltonian")` mostly respects that, but uses `convert_to_legacy_H` if the generator is a `Sum`, `Prod` or `SProd` even with `new_opmath` is enabled, breaking the convention. **Description of the Change:** We add `convert_to_H`, which converts any arithmetic operator to a conceptual Hamiltonian - either `qml.ops.Hamiltonian` or `qml.ops.LinearCombination`. We use the new `convert_to_H` function in the `qml.generator` method. The `convert_to_legacy_H` function continues to convert specifically to a legacy Hamiltonian regardless of the global setting for `new_opmath`. **Benefits:** Consistency, no one will ask for a generator with `new_opmath` enabled and get a legacy Hamiltonian. --------- Co-authored-by: Mudit Pandey <mudit.pandey@xanadu.ai> Co-authored-by: qottmann <korbinian.kottmann@gmail.com> Co-authored-by: Alex Preciado <alex.preciado@xanadu.ai> Co-authored-by: Thomas R. Bromley <49409390+trbromley@users.noreply.github.com>
…#5419) From a [suggestion](#5216 (comment)) in the original PR. [sc-59440]
`hamiltonian.py` defines the legacy `Hamiltonian`. It should not be returning `LinearCombination` in its own methods even with new op math enabled. Co-authored-by: qottmann <korbinian.kottmann@gmail.com>
Qottmann
added a commit
that referenced
this pull request
Mar 25, 2024
…ian` (#5216) Branching: #5269 > #5216 > #5322 > #5335 The basic idea is to have a clone of `qml.Hamiltonian` with a better name, `LinearCombination` - that inherits from `CompositeOp` and plays nice with new opmath. The motivation is that sometimes it is still handy to have an operator class for which you _know_ there is no funny nesting and that has `coeffs` and `ops` separately on demand without processing. ToDo - [x] basic init - [x] `tests/ops/op_math/test_linear_combination.py` pass - [x] update map_wires - [x] all tests pass - [x] add pauli_rep - [x] add operands attribute and make iterable (?) - [x] update string repr - [x] upgrade to internally use new opmath - [x] update tests with default new opmath - [x] dunder math methods support with new opmath - [x] simplify and speed-up `simplify()`, also make sure to not act in-place - [x] Could we make the matrix generation workflow more general like Sum? - [x] Also, deferring to the pauli rep method for matrix generation leads to substantial performance improvements. - [x] toggle `__use_new_opmath` - [x] tests_linear_combination pass - [x] Integration tests - [x] get all linear combination tests to pass locally - [x] utilize super().sparse_matrix - [x] use grouping functionality from Sum that Mudit is adding in #5179 - [x] update matmul to handle other LinearCombination instances - [x] remove top level import - [x] remove unnecessary copies - [x] add diagonalizing gates - [x] bugfix trivial case of simplify empty LinearCombination - [x] add trivial case for is_hermitian - [ ] torch differentiation with simplify=True in constructor - [x] changelog Optional features (likely to be included in a follow-up) - [x] `qml.Hamiltonian` points to either old Hamiltonian or LinearCombination depending on `__use_new_opmath` - [x] support Hermitian - [ ] matrix method (optional) - [ ] adjoint method (optional) - [ ] batching support (optional) - [ ] qml.dot points to LinearCombination - [ ] take care of xfails (mostly about raiseing errors in other parts of the codebase) - [ ] update logic of adjoint differentiation to catch attempt to differentiate lincomb coeffs - [x] support in qml.equal - [x] add special matmul with other product or non-composite ops - [x] add special matmul with other LinearCombination [sc-56704] --------- Co-authored-by: lillian542 <38584660+lillian542@users.noreply.github.com> Co-authored-by: Astral Cai <astral.cai@xanadu.ai> Co-authored-by: Mudit Pandey <mudit.pandey@xanadu.ai> Co-authored-by: Christina Lee <christina@xanadu.ai> Co-authored-by: Pietropaolo Frisoni <pietropaolo.frisoni@xanadu.ai> Co-authored-by: albi3ro <chrissie.c.l@gmail.com> Co-authored-by: Utkarsh <utkarshazad98@gmail.com> Co-authored-by: lillian542 <Lillian.frederiksen@xanadu.ai> Co-authored-by: Alex Preciado <alex.preciado@xanadu.ai> Co-authored-by: Thomas R. Bromley <49409390+trbromley@users.noreply.github.com> Co-authored-by: Vincent Michaud-Rioux <vincentm@nanoacademic.com> Co-authored-by: Josh Izaac <josh146@gmail.com> Co-authored-by: Nathan Killoran <co9olguy@users.noreply.github.com> Co-authored-by: Matthew Silverman <matthews@xanadu.ai> Co-authored-by: Mikhail Andrenkov <mikhail@xanadu.ai> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Qottmann
added a commit
that referenced
this pull request
Mar 25, 2024
PR to toggle the `__use_new_opmath` and make it default. This will require several changes in the codebase (and further updates to demos, dataset and plugins). Bigger changes should be offloaded in separate PRs. Small updates can be gathered here. Branching: #5269 > #5216 > #5322 > #5335 - [ ] Make sure lightning works ```python import os, sys os.environ["OMP_NUM_THREADS"]="8" import pennylane as qml from pennylane import numpy as np from timeit import default_timer as timer if __name__ == "__main__": rng = np.random.default_rng(seed=1337) # make the results reproducable mol = qml.data.load("qchem", molname="H2O", bondlength=0.958, basis="STO-3G")[0] hf_state = mol.hf_state ham = mol.hamiltonian wires = ham.wires dev = qml.device("lightning.qubit", wires=wires, batch_obs=True) n_electrons = mol.molecule.n_electrons singles, doubles = qml.qchem.excitations(n_electrons, len(wires)) hf_state.requires_grad = False hf_ops = [] for idx,i in enumerate(hf_state): if i==1: hf_ops.append((qml.PauliX, idx)) @qml.qnode(dev, diff_method="adjoint") def cost(weights): for (x,i) in hf_ops: # HF state x(i) for idx,s in enumerate(singles): qml.SingleExcitation(weights[idx], wires=s) for idx,d in enumerate(doubles): qml.DoubleExcitation(weights[idx+len(singles)], wires=d) return qml.expval(ham) params = qml.numpy.array(rng.normal(0, np.pi, len(singles) + len(doubles))) opt = qml.GradientDescentOptimizer(stepsize=0.5) # store the values of the circuit parameter angle = [params] max_iterations = 50 procs = int(os.getenv("PL_FWD_BATCH", "0")) pre_s = f"qubits={len(wires)},num_terms={len(ham.terms()[0])},procs={procs}," energies = [] for n in range(max_iterations): start_grad = timer() params, prev_energy = opt.step_and_cost(cost, params) energies.append(prev_energy) end_grad = timer() angle.append(params) print(f"{pre_s},Step={n},Time_grad={end_grad-start_grad}") start_fwd = timer() energy = cost(params) energies.append(energy) end_fwd = timer() print(f"Energies={energies},Time_fwd={end_fwd - start_fwd}") ``` --------- Co-authored-by: lillian542 <38584660+lillian542@users.noreply.github.com> Co-authored-by: Astral Cai <astral.cai@xanadu.ai> Co-authored-by: Mudit Pandey <mudit.pandey@xanadu.ai> Co-authored-by: Christina Lee <christina@xanadu.ai> Co-authored-by: Pietropaolo Frisoni <pietropaolo.frisoni@xanadu.ai> Co-authored-by: albi3ro <chrissie.c.l@gmail.com> Co-authored-by: Utkarsh <utkarshazad98@gmail.com> Co-authored-by: lillian542 <Lillian.frederiksen@xanadu.ai> Co-authored-by: Alex Preciado <alex.preciado@xanadu.ai> Co-authored-by: Thomas R. Bromley <49409390+trbromley@users.noreply.github.com> Co-authored-by: Vincent Michaud-Rioux <vincentm@nanoacademic.com> Co-authored-by: Josh Izaac <josh146@gmail.com> Co-authored-by: Nathan Killoran <co9olguy@users.noreply.github.com> Co-authored-by: Matthew Silverman <matthews@xanadu.ai> Co-authored-by: Mikhail Andrenkov <mikhail@xanadu.ai> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Update tests to pass under new opmath using a context manager and fixture
Branching: #5269 > #5216 > #5322 > #5335
tests/ops/op_mathpass *see caveat below intest_exp.pytests/ops/qubit/pass (includes hamiltonian tests)tests/ops/pass *except some blockers in tests/ops/functions/test_assert_valid.pytests/pulsepass *see pickle blocker, could be localtests/tape/tests/tape/tests/resource/tests/qnn/tests/qinfo/tests/numpy/tests/logging/tests/kernels/tests/fermi/tests/fourier/tests/math/Note that there are currently some tests and parts of the codebase that enable/disable new opmath, which affects other tests by changing the global variable
__use_new_opmathBlockers:
test_exp.py::TestDecomposition::test_generator_decompositiontests/ops/functions/test_generator.pytests/ops/functions/test_is_commuting.pyrequire changes inqml.is_commutingtests/ops/functions/test_assert_valid.py::test_generated_list_of_opstests/ops/functions/test_assert_valid.py::test_explicit_list_of_opstests/ops/functions/test_equal.py::TestObservablesComparisonspass also for LinearCombinationtests/ops/functions/test_equal.pyE AttributeError: Can't pickle local object 'test_standard_validity.<locals>.f1'[sc-57841]