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

Mac os CI pipeline #3318

Closed
wants to merge 25 commits into from
Closed

Mac os CI pipeline #3318

wants to merge 25 commits into from

Conversation

schnellerhase
Copy link
Contributor

@schnellerhase schnellerhase commented Jul 18, 2024

Adds a new mac OS CI pipeline, that should be fast enough to run with every commit/PR.

  • The CI run both a petsc-free and a build with petsc using the brew package

  • Currently we can not verify the python package in the petsc build, as I have no idea how to make petsc4py work properly with the brew package (Note: same holds true for the debian packages and thus restricts the running with debian petsc install)

  • There are two failing python tests in the petsc free run, not sure what's going on there, and whats causing this. Needs to be fixed before merge.

Fixes #3312

based on brew packages
@jhale
Copy link
Member

jhale commented Jul 18, 2024

Looks good. I think this could be an adjustment of the existing macOS pipeline; you could have a variable defining the type of PETSc build, e.g. "none", "brew" or "self".

@schnellerhase
Copy link
Contributor Author

schnellerhase commented Jul 18, 2024

I couldn't figure out how to maintain both the scheduling of the other macos pipeline and allow the non scheduled new runs in the same file.

@schnellerhase
Copy link
Contributor Author

schnellerhase commented Jul 18, 2024

The failing tests are fixed now, was caused by cffi_utils failing if petsc4py is not importable before registering numba types.

@schnellerhase schnellerhase force-pushed the macos_ci branch 2 times, most recently from f8a8c06 to d780b5c Compare July 19, 2024 08:11
@garth-wells
Copy link
Member

Rather than using Homebrew, how about using conda to install dependencies? The conda PETSc package has more options enabled, e.g. PETSc is configure with parallel LU solver. And petsc4py is available.

See #3319.

@jhale
Copy link
Member

jhale commented Jul 22, 2024

I agree with @garth-wells , homebrew's petsc is not really useful for our users so it seems a waste to test against it. Although good to see it's now being built against HDF5 MPI!

I would propose:

  • conda petsc (on commits)
  • no petsc (on commits)
  • homebrew + petsc build (weekly)

@schnellerhase
Copy link
Contributor Author

schnellerhase commented Jul 24, 2024

I can not get the basix build to pass, pretty sure its something to do with the availability of a C++ standard library, however I'm not really sure how to make that available in the conda setting. See here for the basix standalone failure.

@schnellerhase
Copy link
Contributor Author

The error I get is

  Undefined symbols for architecture arm64:
    "std::exception::what() const", referenced from:

Never worked on ARM before - do you guys know what else I need to change to make that work?

@schnellerhase
Copy link
Contributor Author

schnellerhase commented Aug 7, 2024

Okay I guess we have no real fix for this problem for now, so should we return to the case of the brew install and get this merged for now to have a macos pipeline running at all?

Also we could in continuation of #3335 make use of the sub-optimal setting of being able to make petsc run and not petsc4py by compiling the cpp library with petsc support and the python one without support of petsc4py and then run the tests. This is so far not tested in any CI.

@schnellerhase
Copy link
Contributor Author

The cffi fixes here we should maybe extract and merge independent of the macos pipeline. However just noted they are broken, got lost in some merge conflict.

@jhale
Copy link
Member

jhale commented Sep 17, 2024

The cffi fixes here we should maybe extract and merge independent of the macos pipeline. However just noted they are broken, got lost in some merge conflict.

They are at acd43c7 - could you make a new PR with just this?

@jhale jhale closed this Sep 22, 2024
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.

Enable macOS CI on PRs (PETSc-less)
3 participants