Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rebuild for python39 #46

Merged

Conversation

regro-cf-autotick-bot
Copy link
Contributor

This PR has been triggered in an effort to update python39.

Notes and instructions for merging this PR:

  1. Please merge the PR only after the tests have passed.
  2. Feel free to push to the bot's branch to update this PR if needed.

Please note that if you close this PR we presume that the feedstock has been rebuilt, so if you are going to perform the rebuild yourself don't close this PR until the your rebuild has been merged.

This package has the following downstream children:

And potentially more.

If this PR was opened in error or needs to be updated please add the bot-rerun label to this PR. The bot will close this PR and schedule another one. If you do not have permissions to add this label, you can use the phrase @conda-forge-admin, please rerun bot in a PR comment to have the conda-forge-admin add it for you.

This PR was created by the regro-cf-autotick-bot.
The regro-cf-autotick-bot is a service to automatically track the dependency graph, migrate packages, and propose package version updates for conda-forge. If you would like a local version of this bot, you might consider using rever. Rever is a tool for automating software releases and forms the backbone of the bot's conda-forge PRing capability. Rever is both conda (conda install -c conda-forge rever) and pip (pip install re-ver) installable.
Finally, feel free to drop us a line if there are any issues!
This PR was generated by https://github.com/regro/autotick-bot/actions/runs/475217140, please use this URL for debugging

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@h-vetinari
Copy link
Member

@conda-forge/core
I don't understand how this PR can have a green CI, when it clearly depends on a feedstock that hasn't had its python 3.9 builds completed yet:

outputs:
  - name: cvxpy
    requirements:
      host:
        - python
      run:
        - python
        - {{ pin_subpackage('cvxpy-base', exact=True) }}
        - osqp
        - ecos >=2
        - scs >=1.1.3

The build section for cvxpy looks like this:

Packaging cvxpy
INFO:conda_build.build:Packaging cvxpy
INFO conda_build.build:build(2214): Packaging cvxpy
No files or script found for output cvxpy
WARNING:conda_build.build:No files or script found for output cvxpy
WARNING conda_build.build:bundle_conda(1553): No files or script found for output cvxpy
number of files: 0
Fixing permissions
Packaged license file/s.
INFO :: Time taken to mark (prefix)
        0 replacements in 0 files was 0.00 seconds

@h-vetinari h-vetinari marked this pull request as draft January 10, 2021 18:58
Copy link
Member

@h-vetinari h-vetinari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pending resolution of this comment

@beckermr
Copy link
Member

Well if the tests don't call the dependent package, then they will pass.

@h-vetinari
Copy link
Member

Well if the tests don't call the dependent package, then they will pass.

Thanks for the quick response. But I really don't understand - this should already fail when trying to resolve the dependencies of the outputs, if one of the dependencies hasn't been built yet (and this happens all the time for migrations, but somehow not here, which is why I pinged).

@beckermr
Copy link
Member

Did it actually try to solve the envs? Without a build script it may not have.

@rileyjmurray
Copy link
Contributor

@h-vetinari and @beckermr, I'm not really familiar with the build process for conda-forge, but if CVXPY's standard test suite is run, then OSQP will be called. In particular, the line

- nosetests cvxpy
in our conda recipe should trigger dozens of unittests against OSQP. I could be misunderstanding, though.

@h-vetinari
Copy link
Member

Hey @rileyjmurray, thanks for chiming in. I agree with you that this should show up in the test suite.

It's very weird that it didn't run:

TEST START: /home/conda/feedstock_root/build_artifacts/linux-64/cvxpy-1.1.7-py39he80948d_1.tar.bz2
Nothing to test for: /home/conda/feedstock_root/build_artifacts/linux-64/cvxpy-1.1.7-py39he80948d_1.tar.bz2
Renaming work directory '/home/conda/feedstock_root/build_artifacts/cvxpy_1610264372302/work' to '/home/conda/feedstock_root/build_artifacts/cvxpy_1610264372302/work_moved_cvxpy-1.1.7-py39he80948d_1_linux-64_main_build_loop'
INFO:conda_build.utils:Renaming work directory '/home/conda/feedstock_root/build_artifacts/cvxpy_1610264372302/work' to '/home/conda/feedstock_root/build_artifacts/cvxpy_1610264372302/work_moved_cvxpy-1.1.7-py39he80948d_1_linux-64_main_build_loop'
INFO conda_build.utils:shutil_move_more_retrying(2084): Renaming work directory '/home/conda/feedstock_root/build_artifacts/cvxpy_1610264372302/work' to '/home/conda/feedstock_root/build_artifacts/cvxpy_1610264372302/work_moved_cvxpy-1.1.7-py39he80948d_1_linux-64_main_build_loop'
shutil.move(work)=/home/conda/feedstock_root/build_artifacts/cvxpy_1610264372302/work, dest=/home/conda/feedstock_root/build_artifacts/cvxpy_1610264372302/work_moved_cvxpy-1.1.7-py39he80948d_1_linux-64_main_build_loop)
INFO:conda_build.utils:shutil.move(work)=/home/conda/feedstock_root/build_artifacts/cvxpy_1610264372302/work, dest=/home/conda/feedstock_root/build_artifacts/cvxpy_1610264372302/work_moved_cvxpy-1.1.7-py39he80948d_1_linux-64_main_build_loop)
INFO conda_build.utils:shutil_move_more_retrying(2091): shutil.move(work)=/home/conda/feedstock_root/build_artifacts/cvxpy_1610264372302/work, dest=/home/conda/feedstock_root/build_artifacts/cvxpy_1610264372302/work_moved_cvxpy-1.1.7-py39he80948d_1_linux-64_main_build_loop)
# Automatic uploading is disabled
# If you want to upload package(s) to anaconda.org later, type:
[...]

This happens across all builds, not just py39

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I wanted to let you know that I linted all conda-recipes in your PR (recipe) and found some lint.

Here's what I've got...

For recipe:

  • The extra section contained an unexpected subsection name. feedestock-name is not a valid subsection name.

recipe/meta.yaml Outdated Show resolved Hide resolved
@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@h-vetinari h-vetinari force-pushed the rebuild-python39-0-2_heffe8e branch from 802e3b7 to da244f7 Compare January 10, 2021 19:43
@h-vetinari
Copy link
Member

Thanks for the help @isuruf! 😊

@h-vetinari h-vetinari force-pushed the rebuild-python39-0-2_heffe8e branch from da244f7 to e468f58 Compare January 10, 2021 19:56
@rileyjmurray
Copy link
Contributor

@h-vetinari we actually recently changed our testing framework from nosetests to pytest. Could you update the recipe?

@h-vetinari
Copy link
Member

@rileyjmurray: @h-vetinari we actually recently changed our testing framework from nosetests to pytest. Could you update the recipe?

That is excellent news indeed, but I see from cvxgrp/cvxpy@847ae6e that this doesn't appear in a released version yet. I'll do it for the next release then.

@h-vetinari
Copy link
Member

OK, it seems this feedstock hasn't been properly running the test suite for a while - the fix from Isuru now surfaced a pypy error:

ERROR: test_power (cvxpy.tests.test_problem.TestProblem)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/conda/feedstock_root/build_artifacts/cvxpy-split_1610308667270/_test_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_plac/site-packages/cvxpy/tests/test_problem.py", line 1696, in test_power
    self.assertTrue(__builtins__['abs'](1.7*x**.7 - 2.3*x**-3.3 - .45*x**-.55) <= 1e-3)
TypeError: 'module' object is not subscriptable (key 'abs')

@mattip, does this need to be patched somehow (and then upstreamed)?

@h-vetinari
Copy link
Member

Wonderful, this gets better and better... segfaults on windows for python 3.6 and all of ppc64le.

@h-vetinari h-vetinari closed this Jan 10, 2021
@h-vetinari h-vetinari reopened this Jan 10, 2021
@h-vetinari h-vetinari marked this pull request as ready for review January 11, 2021 00:02
h-vetinari
h-vetinari previously approved these changes Jan 11, 2021
@h-vetinari
Copy link
Member

@conda-forge/cvxpy, please take a look (even though I'm not sure why win+py36 keeps failing)

I want to merge this soon to resolve the circular dependency with osqp, where I'd like to revert conda-forge/osqp-feedstock@92556a1 ASAP.

@h-vetinari h-vetinari self-requested a review January 11, 2021 00:06
@h-vetinari h-vetinari dismissed their stale review January 11, 2021 00:08

Potentially skip 3.6 on win

@h-vetinari
Copy link
Member

Also two failures on aarch:

+ nosetests cvxpy
.....................................................S......................................................S....SSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSS.......................F.....SSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSS.....................SSSSSSSSSSSSSSSS............................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................SSSSSSSSSSSSSSSSSSSSSSSSSSSSSS.............................S...........S......................................................................F..............................................................................................................................................................................................................................................................................
======================================================================
FAIL: test_ecos_bb_mi_lp_2 (cvxpy.tests.test_conic_solvers.TestECOS_BB)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/drone/src/build_artifacts/cvxpy-split_1610329695524/_test_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_/site-packages/cvxpy/tests/test_conic_solvers.py", line 1343, in test_ecos_bb_mi_lp_2
    StandardTestLPs.test_mi_lp_2(solver='ECOS_BB')
  File "/drone/src/build_artifacts/cvxpy-split_1610329695524/_test_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_/site-packages/cvxpy/tests/solver_test_helpers.py", line 639, in test_mi_lp_2
    sth.verify_objective(places)
  File "/drone/src/build_artifacts/cvxpy-split_1610329695524/_test_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_/site-packages/cvxpy/tests/solver_test_helpers.py", line 129, in verify_objective
    self.tester.assertAlmostEqual(actual, expect, places)
  File "/drone/src/build_artifacts/cvxpy-split_1610329695524/_test_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_/site-packages/cvxpy/tests/base_test.py", line 38, in assertAlmostEqual
    super(BaseTest, self).assertAlmostEqual(a, b, places=places)
AssertionError: 7945.000004659319 != 8373 within 4 places

======================================================================
FAIL: test_sum_matrix (cvxpy.tests.test_dpp.TestDgp)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/drone/src/build_artifacts/cvxpy-split_1610329695524/_test_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_/site-packages/cvxpy/tests/test_dpp.py", line 770, in test_sum_matrix
    np.testing.assert_almost_equal(problem.value, 16)
  File "/drone/src/build_artifacts/cvxpy-split_1610329695524/_test_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_/site-packages/numpy/testing/_private/utils.py", line 595, in assert_almost_equal
    raise AssertionError(_build_err_msg())
AssertionError: 
Arrays are not almost equal to 7 decimals
 ACTUAL: 15.99999975991318
 DESIRED: 16

----------------------------------------------------------------------
Ran 2284 tests in 907.816s

FAILED (SKIP=162, failures=2)

@rileyjmurray
Copy link
Contributor

@h-vetinari the second failure can be fixed just by dialing down the precision requirements. For the first failure, ECOS_BB has known correctness issues (at the level of its C code). The appropriate solution here would simply be to disable test_ecos_bb_mi_lp_2. Perhaps a warning could be printed during the tests that says we're skipping a test because ECOS_BB is known to return an incorrect solution. Is there a unittest decorator we might use for this?

@h-vetinari
Copy link
Member

@h-vetinari the second failure can be fixed just by dialing down the precision requirements. For the first failure, ECOS_BB has known correctness issues (at the level of its C code). The appropriate solution here would simply be to disable test_ecos_bb_mi_lp_2. Perhaps a warning could be printed during the tests that says we're skipping a test because ECOS_BB is known to return an incorrect solution. Is there a unittest decorator we might use for this?

Under which conditions would you want to skip the test? Just for aarch? It probably depends where the following statement is valid:

ECOS_BB has known correctness issues (at the level of its C code).

@rileyjmurray
Copy link
Contributor

Under which conditions would you want to skip the test? Just for aarch? It probably depends where the following statement is valid:

ECOS_BB has known correctness issues (at the level of its C code).

We (meaning myself, @akshayka, and @SteveDiamond) don't know the full extent of the issues with ECOS_BB. There were even a few versions where we removed ECOS_BB from CVXPY. We have since added it back in, but it's only ever called when a user explicitly requests it. The failing ECOS_BB test seen here passes with our continuous integration testing (see footnote*).

If there is a way to skip the failing ECOS_BB test on just aarch, then that would be great. If not, then disabling it altogether is fine. I would prefer to not remove the test outright. Even if the test is disabled on all platforms, we can use that test to print another warning message about using ECOS_BB. What do you think @SteveDiamond @akshayka?


*TravisCI: Ubuntu 16.04 and macOS 10.13 in python 3.7, 3.8, 3.9. AppVeyor: Windows Server 2012 R2, 2016, 2019 in python 3.6 -- 3.9. Our continuous integration testing no longer attempts to build conda distributions (https://github.com/cvxgrp/cvxpy/pull/1202), so there is a chance some ECOS_BB funniness is happening when packaging for conda.

@h-vetinari
Copy link
Member

If there is a way to skip the failing ECOS_BB test on just aarch, then that would be great. If not, then disabling it altogether is fine. I would prefer to not remove the test outright. Even if the test is disabled on all platforms, we can use that test to print another warning message about using ECOS_BB. What do you think @SteveDiamond @akshayka?

Here I just added the skip for aarch now (using selectors in the recipe rather than logic in the patch itself). I'm happy to upstream a patch, but I'll have to adapt it anyway, as you switched to pytest now.

@h-vetinari h-vetinari force-pushed the rebuild-python39-0-2_heffe8e branch from 87a4765 to e0471f8 Compare January 12, 2021 06:30
@h-vetinari
Copy link
Member

@conda-forge/cvxpy

Something weird was going on on windows + py<38, where the testsuite ran out of memory somewhere in the testsuite:

OSError: [WinError 1455] The paging file is too small for this operation to complete

Once I made the test suite more verbose, I could narrow this down to TestBenchmarks, but then it didn't look like OOM anymore, but more like a segfault. It wasn't clear to me from the log where the error was happening, and nothing stood out in terms of (parameter) size in the tests.

Before going over things with a more fine-grained comb, I'm proposing to skip TestBenchmarks on windows and py<38 for the moment, which passes the test suite - WDYT?

@h-vetinari h-vetinari force-pushed the rebuild-python39-0-2_heffe8e branch from db44653 to e0471f8 Compare January 12, 2021 09:11
@rileyjmurray
Copy link
Contributor

rileyjmurray commented Jan 12, 2021

@h-vetinari it's fine to skip TestBenchmarks on certain platforms as needed. As its name suggests, those tests are there for us to detect regressions in CVXPY's runtime or memory requirements, rather than code correctness.

Edit: There has been a report of segfaults when solving large SDPs (https://github.com/cvxgrp/cvxpy/issues/1132). TestBenchmarks solves an SDP with matrix variable of order n=100, that's not really "large", but it's not small either. How much memory do the Windows build machines have?

@h-vetinari
Copy link
Member

@rileyjmurray: @h-vetinari it's fine to skip TestBenchmarks on certain platforms as needed. As its name suggests, those tests are there for us to detect regressions in CVXPY's runtime or memory requirements, rather than code correctness.

Thanks, sounds good.

@h-vetinari
Copy link
Member

How much memory do the Windows build machines have?

I actually don't know that, maybe @conda-forge/core can provide some info. 🙃

@beckermr
Copy link
Member

The windows machines are standard hosted azure VMs so i'd check the azure docs.

@chrisburr
Copy link
Member

I think it's 7GB for Linux and Windows. See https://docs.microsoft.com/en-us/azure/devops/pipelines/agents/hosted?view=azure-devops&tabs=yaml#hardware

@h-vetinari
Copy link
Member

Thanks both :)

@h-vetinari h-vetinari merged commit dbe45ee into conda-forge:master Jan 12, 2021
@rileyjmurray
Copy link
Contributor

Leaving one final note here, in case we follow-up on the segfault: our TravisCI build VMs run with only 3GB RAM, and AppVeyor runs with between 6GB and 7.5GB RAM.

@regro-cf-autotick-bot regro-cf-autotick-bot deleted the rebuild-python39-0-2_heffe8e branch January 12, 2021 17:13
@h-vetinari h-vetinari mentioned this pull request Jan 4, 2023
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants