Fixing CI

[madhavajay]

Description

Fixing CI.

Affected Dependencies

None

How has this been tested?

Locally and with CI.

Checklist

• [x] I have followed the Contribution Guidelines and Code of Conduct
• [x] I have commented my code following the OpenMined Styleguide
• [x] I have labeled this PR with the relevant Type labels
• [x] My changes are covered by tests

[review-notebook-app[bot]]

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

---

Powered by ReviewNB

[chinmayshah99]

Does this also solve #358 ?

[madhavajay]

Does this also solve #358 ?

Kind of. So it will build and install in CI, but really we should make sure that the wheel it creates can also install on any version of MacOS. There might be some extra tricks to making that happen, and we will likely need to add a second entry to the matrix to be sure:

macOS Big Sur 11.0 | macos-11.0
macOS Catalina 10.15 | macos-latest or macos-10.15

[madhavajay]

I also think this should be done a different way: https://github.com/OpenMined/PyDP/blob/dev/tests/notebooks/test_all_notebooks.py

But before refactoring it would be good to get a clear understanding of the goal? I would highly suggest running linting and checks before committing in pre-commit and then again in the linting step of CI, but currently the pre-commit checks aren't all passing because there are a lot of flake8 issues.

Try this:

$ pre-commit run --all-files
Found 216 errors in 29 files (checked 42 source files)

However, I think they should be handled as a seperate set of tasks so for now the key is just to get the tests passing and the build working so that we can do a Python 3.9 release.

Once these are passing for 3.9 I can add a nightly build to run the older versions like 3.6, 3.7 and 3.8 every night, and a simple file which can be used to trigger them manually by doing something like this:

$ date > tests/trigger/versions

[replomancer]

I only investigated test_bounded_mean.py There may be some issues with other tests (they fail for me). Here's what I found so far.

This test: https://github.com/OpenMined/PyDP/blob/c387752d2b42aebdce52f8fe687d0d9aab828b45/tests/algorithms/test_bounded_mean.py#L69-L73 means if you compute a dp mean of many many many fives, the result should be close to five.

Apparently for performance reasons, instead of building big lists of fives, an attempt is made here to load data: https://github.com/OpenMined/PyDP/blob/c387752d2b42aebdce52f8fe687d0d9aab828b45/tests/algorithms/test_bounded_mean.py#L49-L62

from here: https://github.com/OpenMined/PyDP/blob/c387752d2b42aebdce52f8fe687d0d9aab828b45/tests/algorithms/test_bounded_mean/test_bounded_mean_int64_data.proto#L1-L2 This file looks a little different when something useful is serialized here. Currently instead of having many many fives, BoundedMean probably gets nothing and produces random results between lower and upper bounds. This test passes after data file is deleted but it takes almost 2 minutes on my machine.

~Speaking of lower and upper bounds. Maybe something builds differently for me or somehow I downloaded another C++ lib version but for me this is also broken:~ (this was a result of some dev changes I was missing) https://github.com/OpenMined/PyDP/blob/c387752d2b42aebdce52f8fe687d0d9aab828b45/tests/algorithms/test_bounded_mean.py#L47 ~i.e. I get~

E       RuntimeError: LInf sensitivity has to be positive but is 0

~because~

        x = BoundedMean(1.0, 0, 0, 10, dtype="int64")

~means~

        # lower_bound same as upper_bound:
        x = BoundedMean(epsilon=1.0, lower_bound=0, upper_bound=0, l0_sensitivity=10, dtype="int64")

~I also get the same sensitivity error from here:~ https://github.com/OpenMined/PyDP/blob/c387752d2b42aebdce52f8fe687d0d9aab828b45/tests/algorithms/test_bounded_mean.py#L77-L78 ~I need to increase upper_bound for it to stop crashing.~

I may have ideas/comments tomorrow.

[replomancer]

(edit: this was from missing some dev changes) ~Other fails I see also result from sensitivity set to zero (LInf sensitivity is computed from other parameters) e.g.~ https://github.com/OpenMined/PyDP/blob/c387752d2b42aebdce52f8fe687d0d9aab828b45/tests/algorithms/test_bounded_variance.py#L6-L11 ~Here~

BoundedVariance(epsilon, delta, lower_bound, upper_bound, dtype="float") 

~actually means~

BoundedVariance(epsilon=epsilon, lower_bound=delta, upper_bound=lower_bound, l0_sensitivity=upper_bound, dtype="float") 

~so both lower_bound and upper_bound are set to 0.~

~Similar story in TestBoundedStandardDeviation and TestBoundedSum.~

[madhavajay]

Versions less than 3.9 are now building correctly and all tests are passing: https://github.com/OpenMined/PyDP/actions/runs/875497191