[blas] Resolve baseline problems #38467
Conversation
BillyONeal
changed the title
| blas-test:x64-osx=pass # accelerate framework | ||
| lapack-test:x64-osx=pass # accelerate framework |
There was a problem hiding this comment.
I think you should group the passes separately. Also the comment does not matter here.
There was a problem hiding this comment.
The problem with making those separate is that I feel I would need to explain the expected selection on each triplet twice
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
JonLiu1993
added
info:internal
|
So this is my analysis: a) Please don't add unnecessary So from that analyzes the following should be applied: |
|
Furthermore: If Android uses GCC then the Fortran flags for Android are probably not being setup by the Android toolchain? So switching to clapack could solve that issue maybe. |
I don't know. I am making ci.baseline.txt consistent with the status quo so that people editing other PRs don't experience baseline issues, I'm not trying to improve Android support right now. The status quo I'm looking at is Friday's build: https://dev.azure.com/vcpkg/public/_build/results?buildId=102383&view=results Installing 207/2154 lapack-reference[blas-select,core,noblas]:[email protected]#5...
Building lapack-reference[blas-select,core,noblas]:[email protected]#5...
-- Downloading https://github.com/Reference-LAPACK/lapack/archive/v3.11.0.tar.gz -> Reference-LAPACK-lapack-v3.11.0.tar.gz...
-- Extracting source /vcpkg/downloads/Reference-LAPACK-lapack-v3.11.0.tar.gz
-- Applying patch cmake-config.patch
-- Applying patch lapacke.patch
-- Applying patch fix_prefix.patch
-- Using source at /mnt/vcpkg-ci/b/lapack-reference/src/v3.11.0-6ae738f586.clean
-- The Fortran compiler identification is unknown
CMake Error at scripts/cmake/vcpkg_find_fortran.cmake:62 (message):
Unable to find a Fortran compiler using 'CMakeDetermineFortranCompiler'.
Please install one (e.g. gfortran) and make it available on the PATH!
Call Stack (most recent call first):
ports/lapack-reference/portfile.cmake:47 (vcpkg_find_fortran)
scripts/ports.cmake:175 (include)
error: building lapack-reference:arm-neon-android failed with: BUILD_FAILED
Elapsed time to handle lapack-reference:arm-neon-android: 1.1 s
Installing 208/2154 lapack:arm-neon-android@2023-06-10...
Building lapack:arm-neon-android@2023-06-10...
error: building lapack:arm-neon-android failed with: CASCADED_DUE_TO_MISSING_DEPENDENCIES
due to the following missing dependencies:
lapack-reference[blas-select]:arm-neon-android
lapack-reference[core]:arm-neon-android
Elapsed time to handle lapack:arm-neon-android: 47.7 usInstalling 206/2253 openblas:[email protected]...
Building openblas:[email protected]...
-- Using cached OpenMathLib-OpenBLAS-v0.3.27.tar.gz.
-- Extracting source /vcpkg/downloads/OpenMathLib-OpenBLAS-v0.3.27.tar.gz
-- Applying patch uwp.patch
-- Applying patch fix-redefinition-function.patch
-- Applying patch install-tools.patch
-- Using source at /mnt/vcpkg-ci/b/openblas/src/v0.3.27-b066c33329.clean
-- Configuring x64-android
CMake Error at scripts/cmake/vcpkg_execute_required_process.cmake:112 (message):
Command failed: /vcpkg/downloads/tools/ninja/1.10.2-linux/ninja -v
Working Directory: /mnt/vcpkg-ci/b/openblas/x64-android-rel/vcpkg-parallel-configure
Error code: 1
See logs for more information:
/mnt/vcpkg-ci/b/openblas/config-x64-android-dbg-CMakeCache.txt.log
/mnt/vcpkg-ci/b/openblas/config-x64-android-rel-CMakeCache.txt.log
/mnt/vcpkg-ci/b/openblas/config-x64-android-out.log
Call Stack (most recent call first):
/mnt/vcpkg-ci/installed/x64-linux/share/vcpkg-cmake/vcpkg_cmake_configure.cmake:252 (vcpkg_execute_required_process)
ports/openblas/portfile.cmake:63 (vcpkg_cmake_configure)
scripts/ports.cmake:175 (include)
error: building openblas:x64-android failed with: BUILD_FAILED
Elapsed time to handle openblas:x64-android: 1.6 s
I want someone who fixes openblas to need to change this to
I think anything that changes anything about which blas/lapack backend is selected needs to evaluate this list; that this was not done in #38097 is why we are having this baseline issue. (I didn't know about this separate handling in ci.baseline.txt when I reviewed over there)
I marked them skip because it is important that blas-test and lapack-test pass without them.
That is not possible. People editing PRs unrelated to blas and lapack should not be getting baseline errors. ci.baseline.txt is a reflection of the status quo, not what we want the world to be.
I agree! But I don't myself have time to do that right now, and blocking unrelated PRs while that is debugged is an unacceptable outcome. |
BillyONeal
added
requires:vcpkg-team-review
|
I would like the other maintainers to consider Neumann-A's feedback above |
No the important part is that they pass selecting the correct blas/lapack implementation. Since
I agree with that but the only observed CI error seems to be in android. So marking: should be enough or not?
No. This only needs to be evaluated for The idea is to mark the final downstream dep as passing since that means all upstream deps pass. Marking everything as pass is just micromanagement. also changing blas/lapack backend should have no visible impact on the test. They still should pass. The only impact it can have is conflicting libraries being installed which would require a selection in the baseline. |
|
@ras0219-msft Before the meeting @JavierMatosD indicated that the supports expression outcome seemed reasonable. (discussion for) We are happy with skipping more than absolutely necessary: (Neumann-A part B) @ras0219-msft : This is morally what we have to do whenever there are alternatives so it isn't that much of a disservice if we do that for the BLAS alternatives. (discussion for) We want supports expressions for blas and lapack if the corresponding tests are broken: (Neumann-A part C) @ras0219-msft: In this case, supports expressions make sense however if for whatever reason supports did not make sense we want ci.baseline.txt to reflect the reality of the baseline, even if that combination might not make sense. (discussion for) We want extra pass for the backends: (Neumann-A part A) @BillyONeal I'm OK with this on the grounds that they don't do anything; they're more of a comment than an actually meaningful directive. I'm not sold on the "but that means if the backend changes I have to edit ci.baseline.txt" because I think any time backend selection changes the list needs to be re-audited. We did not discuss Neumann-A's part D because we agree it should be fixed but we can't block baseline issues on that being fixed. Result action items:
|
…kg/public/_build/results?buildId=102893&view=logs&j=18f767b8-9e55-53aa-9c57-42862973482f&t=c8899ec1-5880-5fd7-4dca-ef3731d2bc7b -- Check if compiler accepts -pthread -- Check if compiler accepts -pthread - no -- Looking for pthread_create in pthreads -- Looking for pthread_create in pthreads - not found -- Looking for pthread_create in pthread -- Looking for pthread_create in pthread - not found CMake Error at /vcpkg/downloads/tools/cmake-3.29.2-linux/cmake-3.29.2-linux-x86_64/share/cmake-3.29/Modules/FindPackageHandleStandardArgs.cmake:230 (message): Could NOT find Threads (missing: Threads_FOUND) Call Stack (most recent call first):
BillyONeal
changed the title
BillyONeal
removed
the
requires:vcpkg-team-review
Extended from that originally authored by @Cheney-W in #38097