Fix parameter shift rule validation, add Python 3.14 support, and update minimum Qiskit version#1
Fix parameter shift rule validation, add Python 3.14 support, and update minimum Qiskit version#1Copilot wants to merge 6 commits intoqiskit_2_supportfrom
Conversation
- Add check to detect when parameters appear in non-linear functions - Parameter shift rule only works for linear/affine parameter dependencies - Updated tests to remove non-linear arccos(x[0]) from test circuits - Added new test to verify non-linear parameters raise ValueError Co-authored-by: MoritzWillmann <44642314+MoritzWillmann@users.noreply.github.com>
- Update minimum Python version to 3.10 - Remove Python 3.9 from CI workflows - Remove autoray pinning for Python 3.9 - Remove old Qiskit version tests (0.46, 1.x) - Update classifiers in pyproject.toml This change is necessary because qiskit-machine-learning 0.9.0 dropped Python 3.9 support when migrating to Qiskit 2.0. Co-authored-by: MoritzWillmann <44642314+MoritzWillmann@users.noreply.github.com>
- Use multiple test values (0.3, 0.7) for non-linearity check - Relax tolerance from 1e-10 to 1e-8 for floating-point precision - Add comment explaining larger tolerance for second derivatives Co-authored-by: MoritzWillmann <44642314+MoritzWillmann@users.noreply.github.com>
| python-version: '3.9' | ||
| python-version: '3.10' | ||
| - name: Install dependencies | ||
| run: | | ||
| python -m venv venv_minimal | ||
| source venv_minimal/bin/activate | ||
| python -m pip install --upgrade pip | ||
| pip install toml autoray==0.7.2 matplotlib-inline==0.1.7 | ||
| pip install toml matplotlib-inline==0.1.7 |
There was a problem hiding this comment.
I would keep the minimal python version as 3.9. This should work with everything from the minimal requirements
There was a problem hiding this comment.
Done. Restored Python 3.9 for minimal requirements test in f7a1442.
| pytest_qiskit_0_46: | ||
| runs-on: ubuntu-latest | ||
|
|
||
| steps: | ||
| - uses: actions/checkout@v5 | ||
| - name: Set up Python | ||
| uses: actions/setup-python@v6 | ||
| with: | ||
| python-version: '3.9' | ||
| - name: Install dependencies | ||
| run: | | ||
| python -m venv venv_qiskit_0_46 | ||
| source venv_qiskit_0_46/bin/activate | ||
| python -m pip install --upgrade pip | ||
| pip install . qiskit==0.46.3 qiskit-ibm-runtime==0.20.0 autoray==0.7.2 | ||
| - name: Test with pytest | ||
| run: | | ||
| source venv_qiskit_0_46/bin/activate | ||
| pip install pytest | ||
| pytest tests/ | ||
| - name: clean | ||
| run: | | ||
| rm -rf venv_qiskit_0_46 | ||
|
|
||
| pytest_qiskit_1: | ||
| runs-on: ubuntu-latest | ||
|
|
||
| steps: | ||
| - uses: actions/checkout@v5 | ||
| - name: Set up Python | ||
| uses: actions/setup-python@v6 | ||
| with: | ||
| python-version: '3.9' | ||
| - name: Install dependencies | ||
| run: | | ||
| python -m venv venv_qiskit_1 | ||
| source venv_qiskit_1/bin/activate | ||
| python -m pip install --upgrade pip | ||
| pip install . qiskit==1.1.2 qiskit-algorithms==0.3.0 qiskit-ibm-runtime==0.27.1 autoray==0.7.2 | ||
| - name: Test with pytest | ||
| run: | | ||
| source venv_qiskit_1/bin/activate | ||
| pip install pytest | ||
| pytest tests/ | ||
| - name: clean | ||
| run: | | ||
| rm -rf venv_qiskit_1 | ||
|
|
There was a problem hiding this comment.
I'm fine with dropping most of these, I would keep one test for Qiskit smaller 2, though
There was a problem hiding this comment.
Done. Added pytest_qiskit_1 test for Qiskit 1.x compatibility in f7a1442.
| python-version: ["3.9", "3.10", "3.11", "3.12", "3.13"] | ||
| python-version: ["3.10", "3.11", "3.12", "3.13"] |
There was a problem hiding this comment.
Dropping 3.9 here is fine (or necessary), maybe consider adding 3.14 though
| python-version: ["3.9", "3.12"] | ||
| python-version: ["3.10", "3.12"] |
There was a problem hiding this comment.
Also here, consider making the larger version 3.14 (or at least 3.13)
There was a problem hiding this comment.
Done. Updated Windows test to use Python 3.14 in f7a1442.
| python-version: ["3.9", "3.12"] | ||
| python-version: ["3.10", "3.12"] |
There was a problem hiding this comment.
Also here, consider making the larger version 3.14 (or at least 3.13)
There was a problem hiding this comment.
Done. Updated macOS test to use Python 3.14 in f7a1442.
.github/workflows/quick_tests.yaml
Outdated
| strategy: | ||
| matrix: | ||
| python-version: ["3.9", "3.10", "3.11", "3.12", "3.13"] | ||
| python-version: ["3.10", "3.11", "3.12", "3.13"] |
There was a problem hiding this comment.
Also here consider adding 3.14
There was a problem hiding this comment.
Done. Added Python 3.14 to quick_tests.yaml in f7a1442.
| "Programming Language :: Python :: 3.10", | ||
| "Programming Language :: Python :: 3.11", | ||
| "Programming Language :: Python :: 3.12", | ||
| "Programming Language :: Python :: 3.13", |
There was a problem hiding this comment.
Add all working versions from tests
There was a problem hiding this comment.
Done. Added Python 3.9-3.14 to classifiers in f7a1442.
pyproject.toml
Outdated
| requires-python = ">=3.9" | ||
| requires-python = ">=3.10" |
There was a problem hiding this comment.
Done. Restored requires-python = ">=3.9" in f7a1442.
| # The parameter appears in the gradient expression, but it might be with coefficient 0 | ||
| # (e.g., p[0]*acos(x[0]) has gradient acos(x[0]) + p[0]*0/... where p[0] appears but doesn't affect the value) | ||
| # We check this by evaluating the gradient at two different values of the parameter | ||
| try: | ||
| # Create test values for all parameters in the gradient | ||
| # Try to use values that avoid common domain errors (e.g., division by zero, sqrt of negative) | ||
| test_values = [0.3, 0.7] # Two different values to test dependency | ||
| gradient_values = [] | ||
|
|
||
| for test_val in test_values: | ||
| test_params = {} | ||
| for param in fac.parameters: | ||
| if param == parameter: | ||
| test_params[param] = test_val | ||
| else: | ||
| test_params[param] = 0.3 # Safe value for most expressions | ||
| gradient_values.append(float(fac.bind(test_params))) | ||
|
|
||
| # If the gradient changes with the parameter, it's non-linear | ||
| # Use slightly relaxed tolerance to account for floating-point precision | ||
| if not np.isclose(gradient_values[0], gradient_values[1], rtol=1e-8, atol=1e-8): | ||
| raise ValueError( | ||
| f"Parameter shift rule cannot be applied to non-linear parameters. " | ||
| f"Parameter '{parameter}' appears in a non-linear function in gate " | ||
| f"'{original_gate.name}' with parameter expression '{original_gate.params[0]}'. " | ||
| f"The gradient '{fac}' depends on the parameter '{parameter}', which violates the " | ||
| f"assumptions of the parameter shift rule." | ||
| ) | ||
| except (ValueError, ZeroDivisionError): | ||
| # If we can't evaluate (e.g., division by zero), assume it's non-linear | ||
| raise ValueError( | ||
| f"Parameter shift rule cannot be applied to non-linear parameters. " | ||
| f"Parameter '{parameter}' appears in a non-linear function in gate " | ||
| f"'{original_gate.name}' with parameter expression '{original_gate.params[0]}'. " | ||
| f"The gradient '{fac}' contains the parameter '{parameter}', which may violate the " | ||
| f"assumptions of the parameter shift rule." | ||
| ) | ||
|
|
There was a problem hiding this comment.
Rather than checking numerically, use sympy functionality if the expression is a polinomial of grade 1 wrt the parameter
There was a problem hiding this comment.
Done. Replaced numerical check with sympy-based approach using second derivative to verify polynomial degree in f7a1442. The check now computes the second derivative w.r.t. the parameter - if it's non-zero, the parameter doesn't enter linearly and an error is raised.
…ynomial check - Keep Python 3.9 minimum version in pyproject.toml - Add Python 3.14 to all test matrices - Restore Python 3.9 for minimal requirements test - Add Qiskit 1.x compatibility test - Update Python classifiers to include 3.9 and 3.14 - Replace numerical gradient check with sympy-based polynomial degree check - Use second derivative to determine if parameter enters gate linearly Co-authored-by: MoritzWillmann <44642314+MoritzWillmann@users.noreply.github.com>
|
@copilot tests seem to fail. I think there might be an issue with the minimal qiskit version since this change introduces the use of targets. Can you check and plan to update the minimal qiskit version required? This probably also has implications on other dependencies (python and packages) |
I've identified the issue. The error occurs in The fix is to update the minimum qiskit version from
Working on the fix now.
Fixed in 16c40ce. Updated minimum versions:
The error was in pennylane_circuit.py line 138-142 where |
- Update qiskit>=1.0.0 (was >=0.45.0) - fixes Target.num_qubits=None bug - Update qiskit-aer>=0.13.0 (was >=0.12.0) - for qiskit 1.0 compatibility - Update qiskit-ibm-runtime>=0.20.0 (was >=0.18.0) - for qiskit 1.0 compatibility - Update documentation to reflect new minimum versions The issue was that qiskit 0.45.0's Target.from_configuration() creates a Target without num_qubits, causing TypeError when transpiling with targets in pennylane_circuit.py. Co-authored-by: MoritzWillmann <44642314+MoritzWillmann@users.noreply.github.com>
Addresses three open issues from PR sQUlearn#326 (Qiskit 2.0 support):
Parameter Shift Rule Validation
Parameter shift rule was silently producing incorrect gradients when parameters appeared in non-linear functions. Now raises
ValueErrorfor expressions like:Implementation: Uses sympy-based approach to check if the gate parameter is a polynomial of degree ≤ 1 with respect to the differentiation parameter. Computes the second derivative - if non-zero, the parameter enters non-linearly and an error is raised. This is more mathematically sound than numerical evaluation.
Test updates: Replaced
arccos(x)encodings with linear encodings in existing tests, addedtest_nonlinear_parameter_errorcoveringarccos,sin, and quadratic cases.Python Version Support
Maintains broad Python compatibility while adding support for the latest Python version:
pyproject.toml:requires-python = ">=3.9"(Python 3.9 support maintained)Minimum Qiskit Version Update
Updated minimum dependency versions to fix CI test failures caused by
Target.num_qubitsbeingNonein older Qiskit versions:>=0.45.0→>=1.0.0(fixesTarget.num_qubits=Nonebug inTarget.from_configuration())>=0.12.0→>=0.13.0(compatibility with Qiskit 1.0+)>=0.18.0→>=0.20.0(compatibility with Qiskit 1.0+)docs/install/install.rst) to reflect new minimum versionsThe issue was that Qiskit 0.45.0's
Target.from_configuration()inpennylane_gates.pycreates a Target without specifyingnum_qubits, resulting inTypeError: 'NoneType' object cannot be interpreted as an integerwhentarget_to_backend_properties()tried to executerange(target.num_qubits). This was fixed in Qiskit 1.0.0 where the Target API was stabilized.✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.