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

Drop setting dl open flags in dolfinx/__init__.py #2836

Merged
merged 2 commits into from
Oct 30, 2023

Conversation

francesco-ballarin
Copy link
Member

@francesco-ballarin francesco-ballarin commented Oct 28, 2023

Opening this to assess question #2835 on CI

Closes #2835

@francesco-ballarin
Copy link
Member Author

All 20 CI runs were successful. Is there any additional CI workflow that would be useful to test, but should be triggered manually?

@garth-wells
Copy link
Member

Could you test without 'no-binary' in the pip install of NumPy, see

# Install numpy via pip. Exclude binaries to avoid conflicts with libblas
,

@francesco-ballarin
Copy link
Member Author

francesco-ballarin commented Oct 29, 2023

OK, so there are four scenarios:

  1. fiddling with dl open flags in dolfinx/__init__.py, and installing numpy with no-binary: this is current main branch, and therefore there is not need to test it here;
  2. not fiddling with dl open flags in dolfinx/__init__.py, and installing numpy with no-binary: tested in 9f5804d;
  3. fiddling with dl open flags in dolfinx/__init__.py, and installing numpy without no-binary: tested in 92a5640;
  4. not fiddling with dl open flags in dolfinx/__init__.py, and installing numpy without no-binary: tested in 8f92275

All four scenarios are successful on CI, so I would conclude that fiddling with dl open flags in dolfinx/__init__.py is not necessary, and that installing numpy with no-binary is not necessary either.

Todo list after this PR is approved, and before merge:

@francesco-ballarin francesco-ballarin marked this pull request as ready for review October 29, 2023 14:34
@jhale
Copy link
Member

jhale commented Oct 29, 2023

This fix/hack has been in a very long time! I'm relatively persuaded by the above experiments: it's (probably) no longer an issue.

If we get reports of issues we can quickly back it out.

@jhale
Copy link
Member

jhale commented Oct 29, 2023

Don't worry about conda and debian, my memory is that this is related to numpy binary wheels and openmpi

@drew-parsons
Copy link
Contributor

drew-parsons commented Oct 29, 2023

Agreed about debian. If it's passing github CI then it's safe for the purposes of this PR to assume that tests will pass in Debian builds.

@francesco-ballarin
Copy link
Member Author

I've carried out 3 of the 4 tasks in my todo list (the remaining one is to clean up the docker repositories together with @jhale). This is now ready to merge.

@francesco-ballarin francesco-ballarin added the housekeeping Tidying and style improvements label Oct 29, 2023
@garth-wells garth-wells added this pull request to the merge queue Oct 30, 2023
Merged via the queue into main with commit 8677820 Oct 30, 2023
57 checks passed
@garth-wells garth-wells deleted the francesco/drop-dl-open-flags branch October 30, 2023 07:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
housekeeping Tidying and style improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Is fiddling with dl open flags still needed in dolfinx/__init__.py?
4 participants