Fixing broken macOS tests and CI #1068

bcm-at-zama · 2024-09-25T12:41:55Z

fix(ci): fixing broken tests on macOS

refs https://github.com/zama-ai/concrete-internal/issues/#861
refs #https://github.com/zama-ai/concrete-internal/issues/861
refs https://github.com/zama-ai/concrete-internal/issues/#879
refs #https://github.com/zama-ai/concrete-internal/issues/879

frontends/concrete-python/tests/execution/test_matmul.py

bcm-at-zama · 2024-09-26T12:52:07Z

@BourgerieQuentin @youben11 , what would you advise please?

for sure, frontends/concrete-python/tests/compilation/test_circuit.py fix is to be added
for frontends/concrete-python/tests/execution/test_matmul.py, I have added a skip for now
for the two remaining issue in the tests/execution/test_tfhers.py (see https://github.com/zama-ai/concrete-internal/issues/861), I could skip as well, or you could fix

frontends/concrete-python/tests/execution/test_tfhers.py

bcm-at-zama · 2024-09-26T13:14:44Z

ok I have switched off for a moment the remaining problems: with that it works on my machine, let's check in the CI

frontends/concrete-python/Makefile

bcm-at-zama · 2024-09-26T13:51:33Z

Hopefully we can have working mac tests with this.

bcm-at-zama · 2024-09-26T15:02:37Z

We have added issues:

        # concrete decryption should work
        decrypted = circuit.decrypt(tfhers_encrypted_result)
>       assert (dtype.decode(decrypted) == function(*sample)).all()  # type: ignore
E       assert False
E        +  where False = <built-in method all of numpy.bool_ object at 0x7f22cce058c0>()
E        +    where <built-in method all of numpy.bool_ object at 0x7f22cce058c0> = 0 == 64.all
E        +      where 0 = <bound method TFHERSIntegerType.decode of <concrete.fhe.tfhers.dtypes.TFHERSIntegerType object at 0x7f20cb4d8ac0>>(array([0, 0, 0, 0]))
E        +        where <bound method TFHERSIntegerType.decode of <concrete.fhe.tfhers.dtypes.TFHERSIntegerType object at 0x7f20cb4d8ac0>> = <concrete.fhe.tfhers.dtypes.TFHERSIntegerType object at 0x7f20cb4d8ac0>.decode
E        +      and   64 = <function <lambda> at 0x7f20cb1460d0>(*[8, 8])

tests/execution/test_tfhers.py:460: AssertionError

in https://github.com/zama-ai/concrete/actions/runs/11054029497/job/30709829273. Flaky test, I imagine, @youben11 ?

bcm-at-zama · 2024-09-26T15:04:43Z

the mac CI was broken, https://github.com/zama-ai/concrete/actions/runs/11054015693/job/30709811618?pr=1068

bcm-at-zama · 2024-09-26T15:06:30Z

Hopefully I fixed it, let me try

bcm-at-zama · 2024-09-30T12:14:50Z

CI is still broken. Per @youben11 it's a env setting problem. @aPere3 do you know maybe?

bcm-at-zama · 2024-10-01T13:22:13Z

Much better, thanks @aPere3 (https://github.com/zama-ai/concrete/actions/runs/11121222171/job/30899873375?pr=1068).

Remains the notebook tests on mac, let me have a look.

bcm-at-zama · 2024-10-01T13:25:43Z

Pushed a fix (hopefully)

frontends/concrete-python/Makefile

bcm-at-zama · 2024-10-02T12:00:53Z

It's still broken, https://github.com/zama-ai/concrete/actions/runs/11142605562/job/30965928567?pr=1068

make: ./scripts/jupyter/jupyter.sh: No such file or directory

What's that

bcm-at-zama · 2024-10-02T14:27:30Z

Now notebooks launch but

  File "/private/var/folders/2m/b5zrnpc16zdb1jbqhxgsbv2m0000gn/T/tmp.ohF2QHu7/.testenv/lib/python3.10/site-packages/nbclient/client.py", line 1009, in async_execute_cell
    raise DeadKernelError("Kernel died") from None
nbclient.exceptions.DeadKernelError: Kernel died
Notebook examples/sha256/sha256.ipynb refresh took 455 seconds and failed

I would say SHA256 is too memory intensive, maybe.

bcm-at-zama · 2024-10-02T14:28:06Z

We could have a way to blacklist some tests for mac.

bcm-at-zama · 2024-10-02T15:24:18Z

macOS CI works!! let me remove the debugging stuff.

bcm-at-zama · 2024-10-02T15:26:12Z

.github/workflows/concrete_python_test_macos.yml

+
+          # Remove one test which is known to fail on mac, since it consumes
+          # too much RAM
+          rm examples/sha256/sha256.ipynb


ok with you @BourgerieQuentin ?

bcm-at-zama · 2024-10-02T15:27:28Z

Finger crossed

bcm-at-zama · 2024-10-02T16:45:59Z

Had this issue: https://github.com/zama-ai/concrete-internal/issues/880

bcm-at-zama · 2024-10-02T16:48:14Z

Had this issue as well: https://github.com/zama-ai/concrete-internal/issues/881

bcm-at-zama · 2024-10-02T16:49:33Z

This is supposed to work, now we have flaky or other issues.

bcm-at-zama · 2024-10-02T16:50:26Z

Relaunching.

bcm-at-zama · 2024-10-07T10:16:36Z

The CI is green, let's merge this @BourgerieQuentin ?

youben11

All good with a minor note

frontends/concrete-python/requirements.txt

- fixing the mac CI - fixing the broken tests and notebooks - fixing z3-solver version

bcm-at-zama · 2024-10-07T11:02:21Z

Rebasing.

bcm-at-zama · 2024-10-07T11:23:55Z

There is a problem, https://github.com/zama-ai/concrete/actions/runs/11214137966/job/31168628506?pr=1068:

   Compiling concrete-fft v0.5.1
   Compiling concrete-ntt v0.2.0
error[E0716]: temporary value dropped while borrowed
   --> /Users/ec2-user/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tfhe-0.8.0/src/shortint/ciphertext/compact_list.rs:122:30
    |
110 |                 let functions = match functions {
    |                     --------- borrow later stored here
...
122 |                     None => &vec![None; output_lwe_ciphertext_list.lwe_ciphertext_count().0],
    |                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^-
    |                              |                                                             |
    |                              |                                                             temporary value is freed at the end of this statement
    |                              creates a temporary value which is freed while still in use
    |
    = note: consider using a `let` binding to create a longer lived value
    = note: this error originates in the macro `vec` (in Nightly builds, run with -Z macro-backtrace for more info)

For more information about this error, try `rustc --explain E0716`.
error: could not compile `tfhe` (lib) due to 1 previous error
make[1]: *** [build] Error 101
make: *** [tfhers-utils] Error 2

youben11 · 2024-10-07T11:29:32Z

There is a problem, https://github.com/zama-ai/concrete/actions/runs/11214137966/job/31168628506?pr=1068:

   Compiling concrete-fft v0.5.1
   Compiling concrete-ntt v0.2.0
error[E0716]: temporary value dropped while borrowed
   --> /Users/ec2-user/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tfhe-0.8.0/src/shortint/ciphertext/compact_list.rs:122:30
    |
110 |                 let functions = match functions {
    |                     --------- borrow later stored here
...
122 |                     None => &vec![None; output_lwe_ciphertext_list.lwe_ciphertext_count().0],
    |                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^-
    |                              |                                                             |
    |                              |                                                             temporary value is freed at the end of this statement
    |                              creates a temporary value which is freed while still in use
    |
    = note: consider using a `let` binding to create a longer lived value
    = note: this error originates in the macro `vec` (in Nightly builds, run with -Z macro-backtrace for more info)

For more information about this error, try `rustc --explain E0716`.
error: could not compile `tfhe` (lib) due to 1 previous error
make[1]: *** [build] Error 101
make: *** [tfhers-utils] Error 2

Try to use latest nightly if it's happening locally

bcm-at-zama · 2024-10-07T12:41:31Z

@youben11 : I am not setting any nightly in particular. If you mean the CI already uses nightlies, maybe we can wait tomorrow.

BourgerieQuentin

cherry-picked will be merged in another PR. But can you reply to the comment thanks

BourgerieQuentin · 2024-10-08T07:04:38Z

frontends/concrete-python/tests/compilation/test_circuit.py

@@ -510,10 +511,11 @@ def f(x, y):  # pylint: disable=unused-argument
 @pytest.mark.dataflow
 def test_dataflow_circuit(helpers):
    """
-    Test execution with dataflow_parallelize=True.
+    Test execution with dataflow_parallelize=True, unless on macOS.


should be skipped thanks to the marker?

no this test is not skipped, we just force dataflow_parallelize=False on mac and it works
the test which is skipped is test_zero_matmul in the other file.

BourgerieQuentin · 2024-10-08T07:05:53Z

frontends/concrete-python/Makefile

@@ -145,6 +145,9 @@ test-notebooks:
 	eval $(shell make silent_cp_activate)
 	./scripts/jupyter/jupyter.sh

+test-notebooks-macos:


Why a specific target for macos?

I removed the eval $(shell make silent_cp_activate), as it's done on pytest-macos eg. I am not sure it's strictly needed but as I had issues, I thought it could be related. Up to you to try without that, but at least, in my settings, it works.

bcm-at-zama · 2024-10-08T07:45:59Z

@BourgerieQuentin : I replied to your questions, tell me if you want more details or help, and feel free to close this PR

cla-bot bot added the cla-signed label Sep 25, 2024

bcm-at-zama force-pushed the fixing_some_macos_issues_861 branch 2 times, most recently from 433fce2 to 293e840 Compare September 26, 2024 10:00

bcm-at-zama commented Sep 26, 2024

View reviewed changes

frontends/concrete-python/tests/execution/test_matmul.py Outdated Show resolved Hide resolved

bcm-at-zama marked this pull request as ready for review September 26, 2024 12:50

bcm-at-zama force-pushed the fixing_some_macos_issues_861 branch from 293e840 to bd41b7b Compare September 26, 2024 13:13

bcm-at-zama commented Sep 26, 2024

View reviewed changes

frontends/concrete-python/tests/execution/test_tfhers.py Outdated Show resolved Hide resolved

bcm-at-zama commented Sep 26, 2024

View reviewed changes

frontends/concrete-python/tests/execution/test_tfhers.py Outdated Show resolved Hide resolved

bcm-at-zama force-pushed the fixing_some_macos_issues_861 branch 2 times, most recently from 0df01e6 to b7b944c Compare September 26, 2024 13:47

bcm-at-zama commented Sep 26, 2024

View reviewed changes

frontends/concrete-python/Makefile Outdated Show resolved Hide resolved

bcm-at-zama force-pushed the fixing_some_macos_issues_861 branch from b7b944c to f82baf9 Compare September 26, 2024 14:09

bcm-at-zama changed the title ~~Fixing broken macOS tests~~ Fixing broken macOS tests and CI Sep 26, 2024

bcm-at-zama force-pushed the fixing_some_macos_issues_861 branch 2 times, most recently from 4ab8bd6 to 4eb5d2d Compare September 27, 2024 11:46

bcm-at-zama force-pushed the fixing_some_macos_issues_861 branch from 15efc35 to b290fd0 Compare October 2, 2024 11:00

bcm-at-zama commented Oct 2, 2024

View reviewed changes

frontends/concrete-python/Makefile Show resolved Hide resolved

bcm-at-zama force-pushed the fixing_some_macos_issues_861 branch 3 times, most recently from 1f101ab to 6bc1a7c Compare October 2, 2024 13:37

bcm-at-zama force-pushed the fixing_some_macos_issues_861 branch from 6bc1a7c to 1239f63 Compare October 2, 2024 14:29

bcm-at-zama force-pushed the fixing_some_macos_issues_861 branch from 1239f63 to 051eded Compare October 2, 2024 15:25

bcm-at-zama commented Oct 2, 2024

View reviewed changes

bcm-at-zama force-pushed the fixing_some_macos_issues_861 branch from 051eded to b3af79d Compare October 2, 2024 15:26

bcm-at-zama force-pushed the fixing_some_macos_issues_861 branch from b3af79d to efb9ee4 Compare October 2, 2024 16:50

bcm-at-zama requested review from aPere3 and BourgerieQuentin October 7, 2024 10:16

youben11 approved these changes Oct 7, 2024

View reviewed changes

frontends/concrete-python/requirements.txt Outdated Show resolved Hide resolved

fix(ci): fixing CI for macOS

a696600

- fixing the mac CI - fixing the broken tests and notebooks - fixing z3-solver version

bcm-at-zama force-pushed the fixing_some_macos_issues_861 branch from efb9ee4 to a696600 Compare October 7, 2024 11:02

BourgerieQuentin requested changes Oct 8, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixing broken macOS tests and CI #1068

Fixing broken macOS tests and CI #1068

bcm-at-zama commented Sep 25, 2024 •

edited

Loading

bcm-at-zama commented Sep 26, 2024

bcm-at-zama commented Sep 26, 2024

bcm-at-zama commented Sep 26, 2024

bcm-at-zama commented Sep 26, 2024

bcm-at-zama commented Sep 26, 2024

bcm-at-zama commented Sep 26, 2024

bcm-at-zama commented Sep 30, 2024

bcm-at-zama commented Oct 1, 2024

bcm-at-zama commented Oct 1, 2024

bcm-at-zama commented Oct 2, 2024

bcm-at-zama commented Oct 2, 2024

bcm-at-zama commented Oct 2, 2024

bcm-at-zama commented Oct 2, 2024

bcm-at-zama Oct 2, 2024

bcm-at-zama commented Oct 2, 2024

bcm-at-zama commented Oct 2, 2024

bcm-at-zama commented Oct 2, 2024

bcm-at-zama commented Oct 2, 2024

bcm-at-zama commented Oct 2, 2024

bcm-at-zama commented Oct 7, 2024

youben11 left a comment

bcm-at-zama commented Oct 7, 2024

bcm-at-zama commented Oct 7, 2024

youben11 commented Oct 7, 2024

bcm-at-zama commented Oct 7, 2024

BourgerieQuentin left a comment

BourgerieQuentin Oct 8, 2024

bcm-at-zama Oct 8, 2024

BourgerieQuentin Oct 8, 2024

bcm-at-zama Oct 8, 2024

bcm-at-zama commented Oct 8, 2024

Fixing broken macOS tests and CI #1068

Are you sure you want to change the base?

Fixing broken macOS tests and CI #1068

Conversation

bcm-at-zama commented Sep 25, 2024 • edited Loading

bcm-at-zama commented Sep 26, 2024

bcm-at-zama commented Sep 26, 2024

bcm-at-zama commented Sep 26, 2024

bcm-at-zama commented Sep 26, 2024

bcm-at-zama commented Sep 26, 2024

bcm-at-zama commented Sep 26, 2024

bcm-at-zama commented Sep 30, 2024

bcm-at-zama commented Oct 1, 2024

bcm-at-zama commented Oct 1, 2024

bcm-at-zama commented Oct 2, 2024

bcm-at-zama commented Oct 2, 2024

bcm-at-zama commented Oct 2, 2024

bcm-at-zama commented Oct 2, 2024

bcm-at-zama Oct 2, 2024

Choose a reason for hiding this comment

bcm-at-zama commented Oct 2, 2024

bcm-at-zama commented Oct 2, 2024

bcm-at-zama commented Oct 2, 2024

bcm-at-zama commented Oct 2, 2024

bcm-at-zama commented Oct 2, 2024

bcm-at-zama commented Oct 7, 2024

youben11 left a comment

Choose a reason for hiding this comment

bcm-at-zama commented Oct 7, 2024

bcm-at-zama commented Oct 7, 2024

youben11 commented Oct 7, 2024

bcm-at-zama commented Oct 7, 2024

BourgerieQuentin left a comment

Choose a reason for hiding this comment

BourgerieQuentin Oct 8, 2024

Choose a reason for hiding this comment

bcm-at-zama Oct 8, 2024

Choose a reason for hiding this comment

BourgerieQuentin Oct 8, 2024

Choose a reason for hiding this comment

bcm-at-zama Oct 8, 2024

Choose a reason for hiding this comment

bcm-at-zama commented Oct 8, 2024

bcm-at-zama commented Sep 25, 2024 •

edited

Loading