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

General tidy and restrict cffi < 1.17 due to static condensation demo issue #3343

Merged
merged 27 commits into from
Aug 9, 2024

Conversation

jhale
Copy link
Member

@jhale jhale commented Aug 9, 2024

Not totally clear why but there is an issue with the static condensation demo, numba and a new version of cffi. @michalhabera you can reproduce this by branching and removing the cffi<1.17 restriction from pyproject.toml.

This PR also tidies up all of the CI pipelines; I would like to ask again, please do not put pip install x y z into pipelines. Please use the optional dependencies feature within pyproject.toml.

I have also split apart cffi registering Numba's complex types and creating cffi PETSc wrappers - you can now use cffi with numba complex numbers without having PETSc installed. This was causing a test failure.

@jhale jhale changed the title CFFI 1.17 seems to have issues with numba. Restrict cffi<1.17 due to issues with numba. Aug 9, 2024
@jhale
Copy link
Member Author

jhale commented Aug 9, 2024

See also:

#3340

@jhale jhale changed the title Restrict cffi<1.17 due to issues with numba. Restrict cffi<1.17 due to issues with numba and general tidy Aug 9, 2024
@jhale jhale changed the title Restrict cffi<1.17 due to issues with numba and general tidy General tidy and restrict cffi < 1.17 due to static condensation demo issue Aug 9, 2024
@jhale jhale enabled auto-merge August 9, 2024 14:26
@jhale jhale added this pull request to the merge queue Aug 9, 2024
Merged via the queue into main with commit 06e90b8 Aug 9, 2024
25 of 27 checks passed
@jhale jhale deleted the jhale/pin-cffi branch August 9, 2024 15:01
@michalhabera
Copy link
Contributor

I can reproduce. The issue seems to come from some changes in CFFI, so the global FFI builder object, i.e. cffi.FFI() does not recognize float _Complex and double _Complex types, call to ffi.typeof("double _Complex") gives

Traceback (most recent call last):
  File "/dolfinx-env/lib/python3.12/site-packages/cffi/model.py", line 58, in get_cached_btype
    BType = ffi._cached_btypes[self]
            ~~~~~~~~~~~~~~~~~~^^^^^^
KeyError: <_cffi_double_complex_t>

What seems to work is if we call typeof("double _Complex") on the FFI of the out-of-line compiled module. I'll prepare a PR.

@michalhabera
Copy link
Contributor

Fix will come with python-cffi/cffi#111, I'd suggest we wait with pinned cffi==1.16.0 in the meantime.

schnellerhase pushed a commit to schnellerhase/dolfinx that referenced this pull request Aug 15, 2024
… issue (FEniCS#3343)

* CFFI 1.17 seems to have issues with numba.

* Clean up RedHat build.

* Can people stop adding optional dependencies in CI runs?

* Tidy.

* Tidy.

* General tidying.

* Fix.

* Remove logguru exception

* Tidy up Dockerfile.

* Fix.

* Install cython for petsc4py build

* Setuptools

* Tweaks.

* Fix.

* Always build docs once and only push if on DOLFINx with main or v* tag.

* Add back documentation artifacts.

* Fix.

* Fix.

* Fix.

* Need these, not installed (!).

* petsc4py needs numpy.

* Split apart numba/cffi and petsc/cffi definitions

* Missing cffi!

* Revert.

* Try unpinning CFFI.

* Does seem to be necessary.

* Improve docstring.
schnellerhase pushed a commit to schnellerhase/dolfinx that referenced this pull request Aug 21, 2024
… issue (FEniCS#3343)

* CFFI 1.17 seems to have issues with numba.

* Clean up RedHat build.

* Can people stop adding optional dependencies in CI runs?

* Tidy.

* Tidy.

* General tidying.

* Fix.

* Remove logguru exception

* Tidy up Dockerfile.

* Fix.

* Install cython for petsc4py build

* Setuptools

* Tweaks.

* Fix.

* Always build docs once and only push if on DOLFINx with main or v* tag.

* Add back documentation artifacts.

* Fix.

* Fix.

* Fix.

* Need these, not installed (!).

* petsc4py needs numpy.

* Split apart numba/cffi and petsc/cffi definitions

* Missing cffi!

* Revert.

* Try unpinning CFFI.

* Does seem to be necessary.

* Improve docstring.
schnellerhase pushed a commit to schnellerhase/dolfinx that referenced this pull request Sep 11, 2024
… issue (FEniCS#3343)

* CFFI 1.17 seems to have issues with numba.

* Clean up RedHat build.

* Can people stop adding optional dependencies in CI runs?

* Tidy.

* Tidy.

* General tidying.

* Fix.

* Remove logguru exception

* Tidy up Dockerfile.

* Fix.

* Install cython for petsc4py build

* Setuptools

* Tweaks.

* Fix.

* Always build docs once and only push if on DOLFINx with main or v* tag.

* Add back documentation artifacts.

* Fix.

* Fix.

* Fix.

* Need these, not installed (!).

* petsc4py needs numpy.

* Split apart numba/cffi and petsc/cffi definitions

* Missing cffi!

* Revert.

* Try unpinning CFFI.

* Does seem to be necessary.

* Improve docstring.
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.

2 participants