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

Fix to failing PETSc compile in tests #76

Open
wants to merge 19 commits into
base: master
Choose a base branch
from

Conversation

Neutrino155
Copy link

@Neutrino155 Neutrino155 commented May 5, 2023

openjournals/joss-reviews#5115

Some of the tests were failing with the error:

Error: Failed building wheel for petsc.

Looking at this merge request: https://gitlab.com/petsc/petsc/-/merge_requests/6333, it seems that the issue is that petsc does not have a pyproject.toml file. It seems the current way to fix this is to anchor to [email protected] and use --no-use-pep517 to actively ask pip to install using a legacy mode.

I found that this still seemed to fail for [email protected] when anchoring to petsc==3.18.4, but this didn't appear to be an issue when anchoring to the latest version petsc==3.19.1.

Unfortunately, the CI still fails to complete and errors with:

Error: The operation was canceled.

I think this is due to the CI tests taking too long (they seem to collectively cut out at 50 minutes).

I think it would be a good idea to try to reduce the time it takes for the tests to complete.

Test seemed to fail for v3.10 but might pass for others. Want to see if petsc and petsc4py successful compile now that pip is pinned to 23.0.1.
Suggestion from warning message: 

```DEPRECATION: petsc4py is being installed using the legacy 'setup.py install' method, because it does not have a 'pyproject.toml' and the 'wheel' package is not installed. pip 23.1 will enforce this behaviour change. A possible replacement is to enable the '--use-pep517' option. Discussion can be found at pypa/pip#8559
Lets see if this one still fails to build petsc!
@matthewcarbone
Copy link
Owner

@Neutrino155 you're a wizard if this works haha, thank you!

Yes the PETSc tests take a while but we're well below the timeout limit of 6 hours, and they have worked in the past. I would love to reduce the amount of time these things take to run but I'm not sure how without compromising some of the tests!

@Neutrino155
Copy link
Author

Neutrino155 commented May 5, 2023

@matthewcarbone almost! Unfortunately seems to be just a partial fix. It still fails for Python versions 3.10 and 3.11! I introduced fail-fast: false into the PETSc yml file to check each Python version separately:

https://github.com/Neutrino155/GGCE/actions/runs/4892412153/jobs/8734137644

It seems to fix the tests for Python versions 3.7, 3.8 and 3.9 but still fails for 3.10 and 3.11.

3.10 seems to refuse to fall back to the legacy mode and reproduces the error:

ERROR: Could not build wheels for petsc, petsc4py, which is required to install pyproject.toml-based projects

3.11 builds petsc and petsc4py fine, but still randomly cancels during the PETSc tests for some reason.

@Neutrino155
Copy link
Author

Neutrino155 commented May 5, 2023

Yes the PETSc tests take a while but we're well below the timeout limit of 6 hours, and they have worked in the past. I would love to reduce the amount of time these things take to run but I'm not sure how without compromising some of the tests!

Is it possible to split up the tests perhaps? Run serial tests takes around 20-30 minutes prior to the PETSc tests. Since these are already checked in the CI without PETSc workflows, is it necessary to reproduce them in the CI+PETSc workflows?

It would be nice for users if there were a "quick" (less than 10 minutes ideally) test suite to go alongside the longer test suites for PETSc etc. This would be beneficial for quicker iterative testing.

@codecov-commenter
Copy link

codecov-commenter commented May 5, 2023

Codecov Report

Merging #76 (cfbb53d) into master (b435782) will increase coverage by 0.14%.
The diff coverage is 100.00%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #76      +/-   ##
==========================================
+ Coverage   79.76%   79.90%   +0.14%     
==========================================
  Files          11       12       +1     
  Lines        1685     1697      +12     
==========================================
+ Hits         1344     1356      +12     
  Misses        341      341              
Impacted Files Coverage Δ
ggce/engine/equations.py 80.86% <ø> (ø)
ggce/engine/system.py 75.12% <ø> (ø)
ggce/engine/terms.py 81.60% <ø> (ø)
ggce/executors/petsc4py/base.py 97.59% <100.00%> (ø)
ggce/executors/petsc4py/solvers.py 91.46% <100.00%> (+0.32%) ⬆️
ggce/executors/solvers.py 89.00% <100.00%> (ø)
ggce/utils/utils.py 50.51% <100.00%> (ø)

... and 1 file with indirect coverage changes

Impacted file tree graph

@Chiffafox
Copy link
Collaborator

It seems to fix the tests for Python versions 3.7, 3.8 and 3.9 but still fails for 3.10 and 3.11.

3.10 seems to refuse to fall back to the legacy mode and reproduces the error:

ERROR: Could not build wheels for petsc, petsc4py, which is required to install pyproject.toml-based projects

Nice catch! This is strange though: I tried the same locally with Py3.10 and Py3.11, and installation falls back to legacy mode with no problems, petsc and petsc4py get installed with no issues. This might be specific to the Github-generated environment.

One issue I'll note is that locally for me, installing mpi4py with pip in Py3.10/3.11 inside a conda environment does not work because of linker problems in conda: a well-known problem. Meanwhile we don't see this coming up in the Github-generated environment at all. There is something unique about the environment being set up here.

@Chiffafox
Copy link
Collaborator

From screwing around a bit in another PR, it seems like --no-binary could be deprecated, which is why it is not "hearing" it in later Python versions. I'm trying to find the correct "new" way of forcing legacy install. Otherwise might have to actually debug the wheel building and get that to work.

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.

4 participants