[cmake] [RFC] rewriting FindMKL

classic Classic list List threaded Threaded
5 messages Options
Sik
Reply | Threaded
Open this post in threaded view
|

[cmake] [RFC] rewriting FindMKL

Sik
Hello CMake users and devs,

In openmeeg we are reviewing our MKL package finder to make it more maintainable for ourselves and others who had adopted our solution. Therefore I'm writing you requesting comments and feedback so that is useful for everyone. 

Here is the project we have set up in order to test it. It has a simple example using MKL and CI for different scenarios. 


Thx

--

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
https://cmake.org/mailman/listinfo/cmake
Reply | Threaded
Open this post in threaded view
|

Re: [cmake] [RFC] rewriting FindMKL

Alexander Neundorf
On 2018 M01 22, Mon 15:16:50 CET Sik wrote:

> Hello CMake users and devs,
>
> In openmeeg we are reviewing our MKL package finder to make it more
> maintainable for ourselves and others who had adopted our solution.
> Therefore I'm writing you requesting comments and feedback so that is
> useful for everyone.
>
> Here is the project we have set up in order to test it. It has a simple
> example using MKL and CI for different scenarios.
>
> https://github.com/openmeeg/findmkl_cmake

that's quite a lot of stuff, I'll try to test itin the next days.

Just a few quick comments:

The MESSAGE() in line 39 is ALL_UPPERCASE.

MKL_ROOT_DIR from line 31 is put into the cache, reusing the same variable
name in line 35 probably works, but the interaction between cache- and non-
cache variables is somewhat complicated, so I would recommend to use different
names.

When searching mkl_cblas.h in line 31, in case the Intel compiler is used, the
install directory of CMAKE_CXX_COMPILER could be used in some way as a hint to
help finding MKL.

I don't like the error message in line 39, I usually prefer using
CMAKE_PREFIX_PATH.

Lines 87 to 95 are not correctly indented, also lines 105 to 112.

Is BLA_STATIC in line 117 correct, or is this a typo ?

Alex

--

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
https://cmake.org/mailman/listinfo/cmake
Reply | Threaded
Open this post in threaded view
|

Re: [cmake] [RFC] rewriting FindMKL

Kai Wolf
I've fixed some (mentioned) issues already and opened up a PR on
GitHub. You might want to check that out, otherwise I'll just delete
my PR.


2018-01-22 22:20 GMT+01:00 Alexander Neundorf <[hidden email]>:

> On 2018 M01 22, Mon 15:16:50 CET Sik wrote:
>> Hello CMake users and devs,
>>
>> In openmeeg we are reviewing our MKL package finder to make it more
>> maintainable for ourselves and others who had adopted our solution.
>> Therefore I'm writing you requesting comments and feedback so that is
>> useful for everyone.
>>
>> Here is the project we have set up in order to test it. It has a simple
>> example using MKL and CI for different scenarios.
>>
>> https://github.com/openmeeg/findmkl_cmake
>
> that's quite a lot of stuff, I'll try to test itin the next days.
>
> Just a few quick comments:
>
> The MESSAGE() in line 39 is ALL_UPPERCASE.
>
> MKL_ROOT_DIR from line 31 is put into the cache, reusing the same variable
> name in line 35 probably works, but the interaction between cache- and non-
> cache variables is somewhat complicated, so I would recommend to use different
> names.
>
> When searching mkl_cblas.h in line 31, in case the Intel compiler is used, the
> install directory of CMAKE_CXX_COMPILER could be used in some way as a hint to
> help finding MKL.
>
> I don't like the error message in line 39, I usually prefer using
> CMAKE_PREFIX_PATH.
>
> Lines 87 to 95 are not correctly indented, also lines 105 to 112.
>
> Is BLA_STATIC in line 117 correct, or is this a typo ?
>
> Alex
>
> --
>
> Powered by www.kitware.com
>
> Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ
>
> Kitware offers various services to support the CMake community. For more information on each offering, please visit:
>
> CMake Support: http://cmake.org/cmake/help/support.html
> CMake Consulting: http://cmake.org/cmake/help/consulting.html
> CMake Training Courses: http://cmake.org/cmake/help/training.html
>
> Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html
>
> Follow this link to subscribe/unsubscribe:
> https://cmake.org/mailman/listinfo/cmake
--

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
https://cmake.org/mailman/listinfo/cmake
Reply | Threaded
Open this post in threaded view
|

Re: [cmake] [RFC] rewriting FindMKL

Luis Caro Campos
In reply to this post by Sik
As a comment, would it be possible to include imported targets that encapsulate include dirs, linker arguments and compile definitions? I would say the CMake community is trying to move away from the old-style find packages that define variables instead of targets.

Regards,
Luis

On Mon, Jan 22, 2018 at 3:16 PM, Sik <[hidden email]> wrote:
Hello CMake users and devs,

In openmeeg we are reviewing our MKL package finder to make it more maintainable for ourselves and others who had adopted our solution. Therefore I'm writing you requesting comments and feedback so that is useful for everyone. 

Here is the project we have set up in order to test it. It has a simple example using MKL and CI for different scenarios. 


Thx

--

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
https://cmake.org/mailman/listinfo/cmake



--

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
https://cmake.org/mailman/listinfo/cmake
Sik
Reply | Threaded
Open this post in threaded view
|

Re: [cmake] [RFC] rewriting FindMKL

Sik
I made a PR based on your comment, is this what you were referring to?


On Tue, Jan 23, 2018 at 11:25 AM Luis Caro Campos <[hidden email]> wrote:
As a comment, would it be possible to include imported targets that encapsulate include dirs, linker arguments and compile definitions? I would say the CMake community is trying to move away from the old-style find packages that define variables instead of targets.

Regards,
Luis

On Mon, Jan 22, 2018 at 3:16 PM, Sik <[hidden email]> wrote:
Hello CMake users and devs,

In openmeeg we are reviewing our MKL package finder to make it more maintainable for ourselves and others who had adopted our solution. Therefore I'm writing you requesting comments and feedback so that is useful for everyone. 

Here is the project we have set up in order to test it. It has a simple example using MKL and CI for different scenarios. 


Thx

--

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
https://cmake.org/mailman/listinfo/cmake

--

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
https://cmake.org/mailman/listinfo/cmake

--

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
https://cmake.org/mailman/listinfo/cmake