-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
[blas][lapack] Reorganize lapack/blas metaport system #21479
[blas][lapack] Reorganize lapack/blas metaport system #21479
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!
All manifest files must be formatted
./vcpkg format-manifest ports/*/vcpkg.json
Diff
diff --git a/ports/faiss/vcpkg.json b/ports/faiss/vcpkg.json
index 45783c6..66d717a 100644
--- a/ports/faiss/vcpkg.json
+++ b/ports/faiss/vcpkg.json
@@ -7,8 +7,8 @@
"license": "MIT",
"supports": "!uwp & !osx & !x86",
"dependencies": [
- "lapack",
"blas",
+ "lapack",
{
"name": "vcpkg-cmake",
"host": true
diff --git a/ports/lapack-reference/vcpkg.json b/ports/lapack-reference/vcpkg.json
index 693d2bd..9e99ad8 100644
--- a/ports/lapack-reference/vcpkg.json
+++ b/ports/lapack-reference/vcpkg.json
@@ -5,6 +5,7 @@
"description": "LAPACK — Linear Algebra PACKage",
"homepage": "http://www.netlib.org/lapack/",
"dependencies": [
+ "blas",
{
"name": "vcpkg-cmake-config",
"host": true
@@ -12,7 +13,6 @@
{
"name": "vcpkg-gfortran",
"platform": "windows"
- },
- "blas"
+ }
]
}
After committing all other changes, the version database must be updated
git add -u && git commit
git checkout 711db4012d436d0fdaa3dac08d7a21273c997302 -- versions
./vcpkg x-add-version --all
Diff
diff --git a/versions/b-/blas.json b/versions/b-/blas.json
index c8b3ade..b08e056 100644
--- a/versions/b-/blas.json
+++ b/versions/b-/blas.json
@@ -1,5 +1,10 @@
{
"versions": [
+ {
+ "git-tree": "d635ae3087b955f6ac63427130d5272d618013b2",
+ "version-string": "1",
+ "port-version": 2
+ },
{
"git-tree": "0d105be9337f4a6f294a7eced2da18e8cdb99051",
"version-string": "1",
diff --git a/versions/baseline.json b/versions/baseline.json
index 84e33d8..33062d6 100644
--- a/versions/baseline.json
+++ b/versions/baseline.json
@@ -490,7 +490,7 @@
},
"blas": {
"baseline": "1",
- "port-version": 1
+ "port-version": 2
},
"blaze": {
"baseline": "3.8",
@@ -1370,7 +1370,7 @@
},
"clapack": {
"baseline": "3.2.1",
- "port-version": 19
+ "port-version": 20
},
"clara": {
"baseline": "1.1.5",
@@ -2070,7 +2070,7 @@
},
"faiss": {
"baseline": "1.7.1",
- "port-version": 0
+ "port-version": 1
},
"fakeit": {
"baseline": "2.0.9",
@@ -3226,11 +3226,11 @@
},
"lapack": {
"baseline": "3",
- "port-version": 2
+ "port-version": 3
},
"lapack-reference": {
"baseline": "3.8.0",
- "port-version": 6
+ "port-version": 7
},
"lastools": {
"baseline": "2020-05-09",
@@ -4870,7 +4870,7 @@
},
"openblas": {
"baseline": "0.3.15",
- "port-version": 0
+ "port-version": 1
},
"opencascade": {
"baseline": "7.6.0",
diff --git a/versions/c-/clapack.json b/versions/c-/clapack.json
index f0700bc..3d437c8 100644
--- a/versions/c-/clapack.json
+++ b/versions/c-/clapack.json
@@ -1,5 +1,10 @@
{
"versions": [
+ {
+ "git-tree": "1bb1d54fdc0c5f85069689bf7f615e1bc3968b1c",
+ "version-string": "3.2.1",
+ "port-version": 20
+ },
{
"git-tree": "bedf61cf9cf765a2c823e1f2427a16af6d8a711e",
"version-string": "3.2.1",
diff --git a/versions/f-/faiss.json b/versions/f-/faiss.json
index 8e9e24b..2e449d3 100644
--- a/versions/f-/faiss.json
+++ b/versions/f-/faiss.json
@@ -1,5 +1,10 @@
{
"versions": [
+ {
+ "git-tree": "03aabe640e31d65dae8cb6ac43243a2c07a74046",
+ "version-semver": "1.7.1",
+ "port-version": 1
+ },
{
"git-tree": "7c8e333f76263e7ca027e52fdf1d14b025be234c",
"version-semver": "1.7.1",
diff --git a/versions/l-/lapack-reference.json b/versions/l-/lapack-reference.json
index 99d6f8a..d50a152 100644
--- a/versions/l-/lapack-reference.json
+++ b/versions/l-/lapack-reference.json
@@ -1,5 +1,10 @@
{
"versions": [
+ {
+ "git-tree": "fe960313e2fa7023f53ad0ca51fffa1d85bc47ef",
+ "version-semver": "3.8.0",
+ "port-version": 7
+ },
{
"git-tree": "10799c7ec42f8369179ba7a8e927235596cb8bb7",
"version-semver": "3.8.0",
diff --git a/versions/l-/lapack.json b/versions/l-/lapack.json
index a4c3326..2401e7d 100644
--- a/versions/l-/lapack.json
+++ b/versions/l-/lapack.json
@@ -1,5 +1,10 @@
{
"versions": [
+ {
+ "git-tree": "2aa8e112cca8ee5a819c2b417d10e7afaf8a7227",
+ "version-string": "3",
+ "port-version": 3
+ },
{
"git-tree": "e52f9dc39357e3e7224273a21c0efaf275f15ae6",
"version-string": "3",
diff --git a/versions/o-/openblas.json b/versions/o-/openblas.json
index b7a5414..3149ae0 100644
--- a/versions/o-/openblas.json
+++ b/versions/o-/openblas.json
@@ -1,5 +1,10 @@
{
"versions": [
+ {
+ "git-tree": "f97d8e79a50a1db0232bac9d4458e3c106ae5905",
+ "version": "0.3.15",
+ "port-version": 1
+ },
{
"git-tree": "2a214e1bac47c70d932caef7d74771c8658b1f7a",
"version": "0.3.15",
You have modified or added at least one portfile where deprecated functions are used.
If you feel able to do so, please consider migrating them to the new functions:
vcpkg_install_cmake
-> vcpkg_cmake_install
(from port vcpkg-cmake
)
vcpkg_build_cmake
-> vcpkg_cmake_build
(from port vcpkg-cmake
)
vcpkg_configure_cmake
-> vcpkg_cmake_configure
(Please remove the option PREFER_NINJA
) (from port vcpkg-cmake
)
vcpkg_fixup_cmake_targets
-> vcpkg_cmake_config_fixup
(from port vcpkg-cmake-config
)
In the ports that use the new function, you have to add the corresponding dependencies:
{
"name": "vcpkg-cmake",
"host": true
},
{
"name": "vcpkg-cmake-config",
"host": true
}
The following files are affected:
ports/blas/portfile.cmake
ports/clapack/portfile.cmake
ports/lapack-reference/portfile.cmake
ports/lapack/portfile.cmake
ports/openblas/portfile.cmake
"description": "Metapackage for packages which provide LAPACK", | ||
"dependencies": [ | ||
{ | ||
"name": "clapack", | ||
"platform": "arm & windows" | ||
"platform": "(arm | staticcrt) & windows" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
uwp is missing here. lapack-reference cannot be used on UWP since the fortran library is missing the app bit.
Also this is too restrictive in the selection for my taste.
staticcrt
is an issue with the fortran compiler setup vcpkg uses not a restriction due to the library itself.
So if i want to use a different fortran compiler I now not only have to supply triplet&toolchain but also a port-overlay for lapack.
What is the teams stance on #19413 an simply switching to ifort? |
I'm just a random user, but while I do use the (reference) blas and cblas features of the current port, they are so buggy that I'm happy to see lapack get simplified. This looks easy enough to extend for my uses. So, thumbs up from me. I'd close #21224 and #20952 as no longer relevant. For what it's worth, the gfortran support is very convenient. |
endif() | ||
|
||
vcpkg_configure_cmake( | ||
SOURCE_PATH ${SOURCE_PATH} | ||
PREFER_NINJA | ||
OPTIONS | ||
"-DUSE_OPTIMIZED_BLAS=${USE_OPTIMIZED_BLAS}" | ||
"-DUSE_OPTIMIZED_BLAS=ON" | ||
"-DCBLAS=${CBLAS}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The CBLAS
variable was deleted, so I presume this should just be OFF
.
This PR will also fix #20952. |
Also fix issue #21224. |
|
||
set(CMAKE_MODULE_PATH ${LAPACK_PREV_MODULE_PATH}) | ||
if(NOT lapack_FOUND) | ||
set(NEW_ARGS "") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to add :
cmake_policy(PUSH)
cmake_policy(SET CMP0057 NEW)
Policy CMP0057 is not set: Support new IN_LIST if() operator. Run "cmake
--help-policy CMP0057" for policy details. Use the cmake_policy command to
set the policy and suppress this warning.
IN_LIST will be interpreted as an operator when the policy is set to NEW.
Since the policy is not set the OLD behavior will be used.
set(LAPACK_LINKER_FLAGS "") | ||
set(LAPACK_LIBRARIES "lapack") | ||
set(LAPACK95_LIBRARIES "") | ||
endif() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cmake_policy(POP)
|
||
set(CMAKE_MODULE_PATH ${LAPACK_PREV_MODULE_PATH}) | ||
if(NOT clapack_FOUND) | ||
set(NEW_ARGS "") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to add :
cmake_policy(PUSH)
cmake_policy(SET CMP0057 NEW)
FYI on this PR trying to pull in
|
-# Delete unnecessary architecture detector CMake file. : Because this project only supports x64 architecture. *G Modify Git Ignore file. : Ignore large model file. *M Modify main source file. : Add new functions. *? Update wiki. !- Ready to temporarily remove Linux support. : Link error using `lapack` on Linux. & microsoft/vcpkg#23109 & microsoft/vcpkg#21479 *@ Modify workflows. : Add denpendencies installation step on Linux. : Modify target triplet.
Closing this PR, since there has been no progress in over a year. Feel free to open a new PR if you are still working on this. |
This PR reconstructs vcpkg's system for BLAS and LAPACK:
blas
andlapack
are metaports. Any other port performingfind_package(BLAS)
orfind_package(LAPACK)
should depend on these ports.clapack
andlapack-reference
are implementations oflapack
. I've removed the "embedded blas" option oflapack-reference
.openblas
is the current implementation ofblas
intel-mkl
could be used to implementblas
, though the user will likely need to provide additional redirecting files (some hypotheticalintel-mkl-blas
port that providesshare/blas/vcpkg-cmake-wrapper.cmake
andlib/pkgconfig/blas.pc
).intel-mkl
because it provides additional API surfaces beyond just BLAS.lapack
to useclapack
when targeting Windows with static crt linkage. This unfortunately does mean thatx64-windows
andx64-windows-static
use different LAPACK backends, but it simplifies the ports and removes the feature policy violation fromlapack-reference
(noblas
andcblas
conflicted).See also #19628 and #19608.
+@Neumann-A & @cenit to chime in and make sure the above all makes sense.