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

Set LDFLAGS and CPPFLAGS for keg-only libomp in CI config #7407

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

Conversation

lagru
Copy link
Member

@lagru lagru commented Apr 30, 2024

Description

Closes #7406. The output while running "brew install libomp" suggests that the mentioned flags need to be set accordingly. So check whether this addresses the issue.

Checklist

Release note

For maintainers and optionally contributors, please refer to the instructions on how to document this PR for the release notes.

...

This job started to fail with

 ../meson.build:1:0: ERROR: Compiler /usr/bin/clang cannot compile programs.

The output while running "brew install libomp" suggests that the
mentioned flags need to be set accordingly. So check whether this
addresses the issue.
@lagru lagru added the 🤖 type: Infrastructure CI, packaging, tools and automation label Apr 30, 2024
@lagru
Copy link
Member Author

lagru commented Apr 30, 2024

Great news the compilation works now. However, the tests fail...

=========================== short test summary info ============================
FAILED measure/_moments.py::skimage.measure._moments.moments_hu
FAILED measure/tests/test_fit.py::test_ellipse_parameter_stability - AssertionError: 
Arrays are not almost equal to 7 decimals
 ACTUAL: 2.356194490192345
 DESIRED: 0.7853981633974483
====== 2 failed, 9088 passed, 3 skipped, 12 warnings in 221.30s (0:03:41) ======

@lagru
Copy link
Member Author

lagru commented Apr 30, 2024

@stefanv do you maybe want to take a look at the failing test_ellipse_parameter_stability?

@jarrodmillman
Copy link
Contributor

Note that macos-latest switched from 12.7.4 to 14.4.1.

@stefanv
Copy link
Member

stefanv commented May 2, 2024

The moments failure is innocuous:

Expected:
    array([0.74537037, 0.35116598, 0.10404918, 0.04064421, 0.00264312,
           0.02408546, 0.        ])
Got:
    array([ 7.45370370e-01,  3.51165981e-01,  1.04049179e-01,  4.06442107e-02,
            2.64312299e-03,  2.40854582e-02, -8.90690725e-21])

@stefanv
Copy link
Member

stefanv commented May 2, 2024

The ellipse failure: the ellipse parameters a and b are correctly estimated (good!). The angle is given as 135 instead of 45. I don't think a pi/2 correction is needed in this case, yet it was added.

        # angle of counterclockwise rotation of major-axis of ellipse
        # to x-axis [eqn. 23] from [2].
        phi = 0.5 * np.arctan((2.0 * b) / (a - c))
        if a > c:
            phi += 0.5 * np.pi

a is definitely larger than c in this case, but what does that comparison do?

The code above is from #2482

I think we should carefully look at https://mathworld.wolfram.com/Ellipse.html#eqn23 too. Question is: why does this pop up now, and only on newer macos / clang compiler? They must have a math library with different trigonometric branching.

The parameter stabilization, just below the above, seems fine:

        # stabilize parameters:
        # sometimes small fluctuations in data can cause
        # height and width to swap
        if width < height:
            width, height = height, width
            phi += np.pi / 2

jarrodmillman added a commit to jarrodmillman/scikit-image that referenced this pull request May 2, 2024
Temporary fix until scikit-image#7407 is resolved
@jarrodmillman
Copy link
Contributor

As a temporary workaround, we can merge #7408. This PR will need to be rebased and revert that commit. Also note that macos-latest is now macos-14-arm64 (it used to be macos-12):
2024-05-02T10:26:42,749505816-07:00

lagru pushed a commit that referenced this pull request May 6, 2024
Temporary fix until #7407 is resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🤖 type: Infrastructure CI, packaging, tools and automation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CI fails on MacOS with "clang cannot compile programs"
3 participants