Repository: qiskit-community/qiskit-machine-learning (language: python)

Issue:
Add M1 image for MacOS for tests
<!--
⚠️ If you do not respect this template, your pull request will be closed.
⚠️ Your pull request title should be short detailed and understandable for all.
⚠️ Also, please add it in the CHANGELOG file under Unreleased section.
⚠️ If your pull request fixes an open issue, please link to the issue.

✅ I have added the tests to cover my changes.
✅ I have updated the documentation accordingly.
✅ I have read the CONTRIBUTING document.
-->

### Summary

#726 documents issues using M2. Github recently made M1 images available and this updates the main workflow to include jobs running under ~3.8~ 3.10 and 3.12 (~lower~ (3.10 is is the min available at present for arm64) and upper Python versions supported).

Adding M1 had been talked about in Optimization and and issue exists there qiskit-community/qiskit-optimization#593 which has a link to the github announce for the M1 capability. Linking the issue to this PR can also inform Opt on how I did it here.

### Details and comments

I marked on OnHold - lets see how this goes. I kinda expect it may fail the tests that #726 does. If failed as expected, I altered them as described below and removed the OnHold as it all passes.

Pin torch==2.2.2 in CI to avoid a UTF-8 error in Windows (temporary bug in Torch)
<!--
⚠️ If you do not respect this template, your pull request will be closed.
⚠️ Your pull request title should be short detailed and understandable for all.
⚠️ Also, please add it in the CHANGELOG file under Unreleased section.
⚠️ If your pull request fixes an open issue, please link to the issue.

✅ I have added the tests to cover my changes.
✅ I have updated the documentation accordingly.
✅ I have read the CONTRIBUTING document.
-->

### Summary
Pins `torch==2.2.2` in CI to avoid a UTF-8 error in Windows. This change is temporary and should be reverted to the latest Torch version as soon as the fix for https://github.com/pytorch/pytorch/issues/124897 reflects in pypi.


Update black (backport #799)


### Summary

Updates black to latest version (older version had a potential denial of service attack)

Similar update to qiskit-community/qiskit-nature#1357

### Details and comments


<hr>This is an automatic backport of pull request #799 done by [Mergify](https://mergify.com).
Pin torch==2.2.2 in CI to avoid a UTF-8 error in Windows (temporary bug in Torch) (backport #796)


### Summary
Pins `torch==2.2.2` in CI to avoid a UTF-8 error in Windows. This change is temporary and should be reverted to the latest Torch version as soon as the fix for https://github.com/pytorch/pytorch/issues/124897 reflects in pypi.

<hr>This is an automatic backport of pull request #796 done by [Mergify](https://mergify.com).


Discussion / hints:
The lowest version arm64 Python is presently 3.10 so I switched to that, as the lower version used, as the 3.8 had before failed since it could not find a compatible Python from the [available set](https://raw.githubusercontent.com/actions/python-versions/main/versions-manifest.json) which indeed there is not presently

It did indeed fail on the unit tests indicated in #726. I added enforce_psd=False to both tests, as they should be PSD since its an ideal statevector computation the sampler uses.

````
==============================
Failed 2 tests - output below:
==============================

test.algorithms.regressors.test_qsvr.TestQSVR.test_qsvr
-------------------------------------------------------

Captured traceback:
~~~~~~~~~~~~~~~~~~~
    Traceback (most recent call last):

      File "/Users/runner/work/qiskit-machine-learning/qiskit-machine-learning/test/algorithms/regressors/test_qsvr.py", line 60, in test_qsvr
    self.assertAlmostEqual(score, 0.38359, places=4)

      File "/Library/Frameworks/Python.framework/Versions/3.12/lib/python3.12/unittest/case.py", line 939, in assertAlmostEqual
    raise self.failureException(msg)

    AssertionError: 0.38411965819305227 != 0.38359 within 4 places (0.0005296581930522848 difference)


test.algorithms.regressors.test_qsvr.TestQSVR.test_change_kernel
----------------------------------------------------------------

Captured traceback:
~~~~~~~~~~~~~~~~~~~
    Traceback (most recent call last):

      File "/Users/runner/work/qiskit-machine-learning/qiskit-machine-learning/test/algorithms/regressors/test_qsvr.py", line 71, in test_change_kernel
    self.assertAlmostEqual(score, 0.38359, places=4)

      File "/Library/Frameworks/Python.framework/Versions/3.12/lib/python3.12/unittest/case.py", line 939, in assertAlmostEqual
    raise self.failureException(msg)

    AssertionError: 0.38411965819305227 != 0.38359 within 4 places (0.0005296581930522848 difference)
```
These are the new M1 jobs - its really fast compared to the other jobs - and I checked the log says it successfully completed the same set of tests.

![image](https://github.com/qiskit-community/qiskit-machine-learning/assets/40241007/22cd9f3b-9063-417c-ad4a-a69b6e461758)

> Machine Learning Unit Tests / MachineLearning (macos-latest, 3.12) (pull_request) Failing after 10m 

Hmmm, this installed Python 3.12.1 for some reason rather than the new 3.12.2 and 3.12.1 causes a known issue with testtools (used by stestr that we use for testing) where this issue was a breaking change in the unit test result that was reverted in 3.12.2. I'll try just re-running the job and see. It passed the nightly CI so unless there is some more recent change - I do not think anything done by  this PR should cause this.

I just re-ran the job and I see in the log this time its installing 3.12.2 as expected. So I have no idea what happened above. I expect it to pass now as it will no longer break in the test infrastructure when skipping tests.
## Pull Request Test Coverage Report for [Build 8332909604](https://coveralls.io/builds/66365044)


### Details

* **0** of **0**   changed or added relevant lines in **0** files are covered.
* No unchanged relevant lines lost coverage.
* Overall coverage remained the same at **92.636%**

---



|  Totals | [![Coverage Status](https://coveralls.io/builds/66365044/badge)](https://coveralls.io/builds/66365044) |
| :-- | --: |
| Change from base [Build 8109418284](https://coveralls.io/builds/66020456): |  0.0% |
| Covered Lines: | 1887 |
| Relevant Lines: | 2037 |

---
##### 💛  - [Coveralls](https://coveralls.io)

This all passes, but as expected the couple of unit tests that we saw fail on an M2 machine, failed here too with M1. I updated them to remove the enforce psd as described above. At this point its ready. If we want to have these jobs Required too, I do not see why we wouldn't, then the Branch Rules need to be updated for that. Since this is only CI, and to keep CI current there in stable, since otherwise I thinks it the same as main, I think adding the StableBackportPotential label would be good to have it backported as well.
It is interesting, but I can't reproduce the issue on M1 and python 3.11. I think, putting `enforce_psd` to false is not a proper way for tests. May be it is better to either reduce the accuracy or replace the tests. If I recall correctly, the tests have been constructed out of a classification problem and the tests are for a regression model.
> It is interesting, but I can't reproduce the issue on M1 and python 3.11. I think, putting enforce_psd to false is not a proper way for tests. May be it is better to either reduce the accuracy or replace the tests.

Strange - we have not really determined why this happens. Its not only M1/M2, where this seems to have surfaced more, as #726 has the same issue with a different OS on x86 This is qiskit.primitives Sampler so the result should be ideal and not need enforce_psd. I can tell you if you replace that by the Aer sampler in ideal mode (shots=None), and give that explicitly to the fidelity so it does not use the default, then the test passes. So its not clear the exact cause of why, in certain environments, this comes up with a different answer. Yes one could reduce the accuracy of the test, but if enforce_psd is not required - it should not be required for an ideal outcome right - that seemed better to me. The fact that with enforce_psd it seems to change things under certain conditions,  presumably a slightly different sampler result,  and causes a different outcome, without further investigation of exactly what is happening and pinning the cause down I guess we will not know.
@oscar-wallis What do you think how we should proceed here? I'm fine either way: merge as it is now, update the tests, or something else.
Let me link #434 which is intended to improve these tests - if we have M1 job in place at that point it will be interesting to see if there is any difference.
> do we need a release note for this?

Release notes are for changes to/around the public API that are of concern to end users. This can include bug fixes of course since these can be perceived when using the public API, and performance. Internal refactoring that does not reflect in public API would not need one, nor do changes to CI like this.
## Pull Request Test Coverage Report for [Build 8923126612](https://coveralls.io/builds/67264191)


### Details

* **0** of **0**   changed or added relevant lines in **0** files are covered.
* No unchanged relevant lines lost coverage.
* Overall coverage remained the same at **92.636%**

---



|  Totals | [![Coverage Status](https://coveralls.io/builds/67264191/badge)](https://coveralls.io/builds/67264191) |
| :-- | --: |
| Change from base [Build 8333263574](https://coveralls.io/builds/66365854): |  0.0% |
| Covered Lines: | 1887 |
| Relevant Lines: | 2037 |

---
##### 💛  - [Coveralls](https://coveralls.io)

It seems the stable branch no longer builds:

- macos-latest is now an ARM image (not an X86 image as it used to be). So the test fix from #779 needs backporting. We could backport the entire PR - also see #801 which is noting macos-latest is now in fact ARM and how to adjust CI
- Windows is failing under mypy which either requires an updated release from torch that corrects the native file or #796 to be backported as a temporary fix.

Shall we temporarily backport https://github.com/qiskit-community/qiskit-machine-learning/pull/796 (and then backport the restored versioning for Torch later on) before backporting this PR?
There are two failures in CI here that require fixes from two PRs, as listed above, applied together as can be seen from #802 which just backported one of them. The other I would be reluctant to backport now since the new CI builds it had added back then, were M1 images, and now as the other issue linked above related to CI and images notes the main Mac images are now M1 and no longer X86 as they always used to be so CI should be updated otherwise at present its running the same stuff twice.

I had thought to do a PR direct to stable with both the test fix for M1 - it needs it here now in stable, since, as I mentioned above, the mac os latest image is M1 rather than X86 here (we never backported the M1 addition with the test fix) - and pin torch. Let me do that so CI passes.




Fix patch:
diff --git a/.github/actions/install-machine-learning/action.yml b/.github/actions/install-machine-learning/action.yml
index 494988b69..17dbee841 100644
--- a/.github/actions/install-machine-learning/action.yml
+++ b/.github/actions/install-machine-learning/action.yml
@@ -1,6 +1,6 @@
 # This code is part of a Qiskit project.
 #
-# (C) Copyright IBM 2021, 2023.
+# (C) Copyright IBM 2021, 2024.
 #
 # This code is licensed under the Apache License, Version 2.0. You may
 # obtain a copy of this license in the LICENSE.txt file in the root directory
@@ -17,6 +17,7 @@ runs:
   using: "composite"
   steps:
     - run : |
+        pip install torch==2.2.2
         pip install -e .[torch,sparse]
         pip install -U -c constraints.txt -r requirements-dev.txt
       shell: bash


Test patch:
diff --git a/test/algorithms/regressors/test_qsvr.py b/test/algorithms/regressors/test_qsvr.py
index a365a3fe1..2f3a6ef21 100644
--- a/test/algorithms/regressors/test_qsvr.py
+++ b/test/algorithms/regressors/test_qsvr.py
@@ -1,6 +1,6 @@
 # This code is part of a Qiskit project.
 #
-# (C) Copyright IBM 2021, 2023.
+# (C) Copyright IBM 2021, 2024.
 #
 # This code is licensed under the Apache License, Version 2.0. You may
 # obtain a copy of this license in the LICENSE.txt file in the root directory
@@ -51,7 +51,7 @@ def setUp(self):
 
     def test_qsvr(self):
         """Test QSVR"""
-        qkernel = FidelityQuantumKernel(feature_map=self.feature_map)
+        qkernel = FidelityQuantumKernel(feature_map=self.feature_map, enforce_psd=False)
 
         qsvr = QSVR(quantum_kernel=qkernel)
         qsvr.fit(self.sample_train, self.label_train)
@@ -61,7 +61,7 @@ def test_qsvr(self):
 
     def test_change_kernel(self):
         """Test QSVR with QuantumKernel later"""
-        qkernel = FidelityQuantumKernel(feature_map=self.feature_map)
+        qkernel = FidelityQuantumKernel(feature_map=self.feature_map, enforce_psd=False)
 
         qsvr = QSVR()
         qsvr.quantum_kernel = qkernel