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

pymepack binding for gem #2166

Open
wants to merge 22 commits into
base: main
Choose a base branch
from

Conversation

almalejko
Copy link

@almalejko almalejko commented Aug 28, 2023

This PR adds a binding to python-mepack (pymepack) backend lyapunov solvers.

The binding offers the option for iterative refinement and shows better performance when compared to slycot.

NOTE: branch for this pull request is based on the branch for #2167 in the forked project.

@github-actions
Copy link
Contributor

Mirroring external branch in external_pr_2166

@almalejko
Copy link
Author

pre-commit.ci autofix

Copy link
Member

@pmli pmli left a comment

Choose a reason for hiding this comment

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

At a quick glance, everything necessary to use pymepack seems to be here. The question is how to test it in CI. How do you install pymepack? For the CI, it would be nice if it was possible to install pymepack using pip from PyPI (i.e., pip install pymepack).

Comment on lines 1 to 2
# This file is part of the pyMOR project (https://www.pymor.org).
# Copyright pyMOR developers and contributors. All rights reserved.
Copy link
Member

Choose a reason for hiding this comment

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

This should be reverted.

Copy link
Author

Choose a reason for hiding this comment

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

pymepack-0.9.1-cp38-cp38-manylinux_2_31_x86_64.zip
@pmli , please, use this wheel to pip install the pymepack project.

@pmli pmli added the pr:new-feature Introduces a new feature label Aug 28, 2023
@pmli pmli self-assigned this Aug 28, 2023
@pmli pmli added this to the 2023.2 milestone Aug 28, 2023
@pmli
Copy link
Member

pmli commented Sep 1, 2023 via email

@almalejko
Copy link
Author

For some reason, I got the above comment by email, but I don't see it in this thread. But, anyway, installing from PyPI would be nicer, but I guess installing from URL could also work in CI.

@pmli , the wheel w/o dependencies, so I deleted the comment.
Please, use the manylinux wheel that I have attached to verify the changes. Pymepack is not yet available at PyPI.

@pmli pmli self-requested a review September 18, 2023 05:58
Copy link
Member

@pmli pmli left a comment

Choose a reason for hiding this comment

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

I can confirm that the pymepack tests pass. However, as I said, it would be good for us to have wheels on PyPI for all Python versions from 3.8 to 3.11.

@almalejko
Copy link
Author

I can confirm that the pymepack tests pass. However, as I said, it would be good for us to have wheels on PyPI for all Python versions from 3.8 to 3.11.

@pmli , the PR is ready for review, pymepack is available at PyPI: https://pypi.org/project/pymepack/

@pmli pmli self-requested a review November 27, 2023 22:03
@github-actions github-actions bot added the infrastructure Buildsystem, packages and CI label Nov 28, 2023
Copy link
Contributor

This pull request modifies pyproject.toml or requirements-ci-oldest-pins.in.
In case dependencies were changed, make sure to call

make ci_requirements

and commit the changed files to ensure that CI runs with the updated dependencies.

@pmli
Copy link
Member

pmli commented Nov 28, 2023

I tried updating the requirements, but running DOCKER=podman make ci_requirements produces the error:

  ERROR: Cannot install dune-xt[visualisation]==2022.5.3, pymor (pyproject.toml), scipy==1.5.4 and scipy>=1.5.4 because these package versions have conflicting dependencies.
Discarding scipy==1.5.4 (from -r requirements-ci-oldest.txt (line 505)) to proceed the resolution
  ERROR: Cannot install dune-xt[visualisation]==2022.5.3, pymor (pyproject.toml), scipy==1.5.4 and scipy>=1.5.4 because these package versions have conflicting dependencies.

@sdrave, any ideas?

@sdrave
Copy link
Member

sdrave commented Nov 28, 2023

I tried updating the requirements, but running DOCKER=podman make ci_requirements produces the error:

  ERROR: Cannot install dune-xt[visualisation]==2022.5.3, pymor (pyproject.toml), scipy==1.5.4 and scipy>=1.5.4 because these package versions have conflicting dependencies.
Discarding scipy==1.5.4 (from -r requirements-ci-oldest.txt (line 505)) to proceed the resolution
  ERROR: Cannot install dune-xt[visualisation]==2022.5.3, pymor (pyproject.toml), scipy==1.5.4 and scipy>=1.5.4 because these package versions have conflicting dependencies.

@sdrave, any ideas?

No idea why the error message has to be so bad and I don't understand what dune-xt hast to do with it. But requirements-ci-oldest-pins.in pins scipy to 1.5.4. pymepack apparently has scipy>=1.6.0.

Copy link
Contributor

This pull request modifies pyproject.toml or requirements-ci-oldest-pins.in.
In case dependencies were changed, make sure to call

make ci_requirements

and commit the changed files to ensure that CI runs with the updated dependencies.

@sdrave
Copy link
Member

sdrave commented Nov 29, 2023

The CI failures seem due to the fact that the pymepack wheel has been compiled for a newer numpy than the one we test with. For binary wheels it is generally a good idea to depend on oldest-supported-numpy. I'm not sure whether this issue can be resolved before hard freeze.

@pmli
Copy link
Member

pmli commented Nov 29, 2023

I'm not sure whether this issue can be resolved before hard freeze.

True. Let's postpone this.

@pmli pmli modified the milestones: 2023.2, 2024.1 Nov 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infrastructure Buildsystem, packages and CI pr:new-feature Introduces a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants