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

FIND_PACKAGE() for BLAS and LAPACK #69

Closed
wants to merge 6 commits into from
Closed

FIND_PACKAGE() for BLAS and LAPACK #69

wants to merge 6 commits into from

Conversation

nschloe
Copy link
Contributor

@nschloe nschloe commented May 19, 2015

This time for BLAS and LAPACK.

@bartlettroscoe bartlettroscoe changed the title More FIND_PACKAGE() FIND_PACKAGE() for BLAS and LAPACK May 21, 2015
@bartlettroscoe
Copy link
Member

I will take a look in a bit here. I am going to ask if there is a Trilinos developer who wants to help test this.

@nschloe
Copy link
Contributor Author

nschloe commented May 22, 2015

Once PR #70 has landed, I'll adapt those here accordingly.

@nschloe
Copy link
Contributor Author

nschloe commented May 31, 2015

I've updated the BLAS and LAPACK logic according to what's in HDF5. Ready to be reviewed.

@bartlettroscoe
Copy link
Member

Sorry I let this set for so long. I meant to send out the below email a long time ago. Since my projects don't use the default find operations for BLAS and LAPACK, I can only test that the overrides work. We really need willing beta tests a chance to test out changes like this. If no one bites, I will just test this for VERA and Trilinos and a default configure trying to find the MLK (which is always a pain to find by default) and if that works, I will merge. The manual testing is where all the work is in accepting a change like this.


From: [email protected] [mailto:[email protected]] On Behalf Of Bartlett, Roscoe A.
Sent: Wednesday, August 12, 2015 2:40 PM
To: Trilinos Developers List ([email protected])
Cc: [email protected]; Nico Schlömer ([email protected])
Subject: [Trilinos-Framework] Better default find for BLAS and LAPACK?

Hello Trilinos Developers,

Are any of you interested in better default find operations for BLAS and LAPACK? Nico has proposed updating the FindTPLBLAS.cmake and FindTPLLAPACK.cmake find modules that use the built-in CMake FindBLAS.cmake and FindLAPACK.cmake modules as the default find.

To try this out, you can do:

$ cd Trilinos/
$ git clone https://github.com/nschloe/TriBITS.git
$ git checkout --track nschloe/more-cmake-find

Then just configure Trilinos with the additional option:

-DTrilinos_TRIBITS_DIR:STRING=TriBITS/tribits

Let me know if you are willing to help test these new TriBITS TPL find modules.

The issue is that we are trying to provide better default find operations without breaking backward compatibility.

Cheers,

-Ross

P.S. Any objection for me sending this out to trilinos-users mail list in case any of them want to help test this? This would be for their benefit, not us really.

@nschloe
Copy link
Contributor Author

nschloe commented Aug 12, 2015

Sounds good.

FWIW, I've successfully tested this on three different machines (all Debian or derivatives, though).

@bartlettroscoe
Copy link
Member

@nschloe, I created the new branch 'better-find-blas-lapack-69' in the main TriBITS repo which merges in this branch which is up to date with the 'master' branch. That should be consistent with Trilinos.

@bartlettroscoe
Copy link
Member

I just send the email below to Trilinos developers and users to test these new find modules out. I don't think much testing will be needed on the CASL VERA side because we don't do default finds anyway.

Now we will just wait and see if we get any takers.

But I will note that one of the reasons the PETSc configure takes so long is that it tests out the various BLAS and LAPACK library candidates to try to find ones that work with the provided compilers, etc. Therefore, if we really are going to make it easy for new developers to find BLAS and LAPACK, we should just build it for them (or give them a 10 minute configure and do what PETSc does).


From: Bartlett, Roscoe A.
Sent: Thursday, August 13, 2015 10:02 AM
To: [email protected]; [email protected]
Cc: [email protected]; Nico Schlömer ([email protected])
Subject: Better default find for BLAS and LAPACK?

Hello Trilinos Users and Developers,

Are any of you interested in better default find operations for BLAS and LAPACK? Nico has proposed updating the FindTPLBLAS.cmake and FindTPLLAPACK.cmake find modules to use the built-in CMake FindBLAS.cmake and FindLAPACK.cmake modules as the default find. But these still allow the standard overrides defined as before to maintain backward compatibility as defined here:

https://tribits.org/doc/TribitsBuildReference.html#enabling-support-for-an-optional-third-party-library-tpl 

To try this out and help us test this, you can do:

$ cd Trilinos/
$ git clone https://github.com/TriBITSPub/TriBITS
$ git checkout --track origin/better-find-blas-lapack-69

Then just configure Trilinos with the additional option:

-DTrilinos_TRIBITS_DIR:STRING=TriBITS/tribits

and see how it works to find BLAS and LAPACK on your machines.

The default find for BLAS and LAPACK should use FIND_PACKAGE(BLAS …) and FIND_PACKAGE(LAPACK …) (if you don’t set any of the BLAS_XXX or LAPACK_XXX variables directing what to find). Consult documentation for the FindBLAS.cmake and FindLAPACK.cmake modules for your version of CMake for details on how to direct these on where to find BLAS and LAPACK using these. If the default find is working, you should see something like:

Processing enabled TPL: BLAS (enabled by TeuchosNumerics, disable with -DTPL_ENABLE_BLAS=OFF)
-- Using FIND_PACKAGE(BLAS ...) ...
-- Looking for Fortran sgemm
-- Looking for Fortran sgemm - found
-- A library with BLAS API found.
-- TPL_BLAS_LIBRARIES='/usr/lib64/libblas.so'
Processing enabled TPL: LAPACK (enabled by TeuchosNumerics, disable with -DTPL_ENABLE_LAPACK=OFF)
-- Using FIND_PACKAGE(LAPACK ...) ...
-- A library with BLAS API found.
-- Looking for Fortran cheev
-- Looking for Fortran cheev - found
-- A library with LAPACK API found.
-- TPL_LAPACK_LIBRARIES='/usr/lib64/liblapack.so;/usr/lib64/libblas.so'

If you do test this out, please comment on this at:

https://github.com/TriBITSPub/TriBITS/pull/69

Otherwise, we will do what testing we can and then push this to TriBITS and then snapshot to Trilinos and other projects.

The issue is that we are trying to provide better default find operations without breaking backward compatibility for projects and customers that rely on standardized TriBITS overrides.

Thanks,

-Ross

P.S. We will need to add specific documentation of which FindTPL.cmake modules are set up to use the provided Find.cmake modules shipped with CMake using the FIND_PACKAGE(…) commands as specified at:

 https://tribits.org/doc/TribitsDevelopersGuide.html#how-to-use-find-package-for-a-tribits-tpl

@nschloe
Copy link
Contributor Author

nschloe commented Aug 13, 2015

Therefore, if we really are going to make it easy for new developers to find BLAS and LAPACK, we should just build it for them (or give them a 10 minute configure and do what PETSc does).

Maybe, but this is not something that needs to be mangled with the build system. It could be handled outside of TriBits in a slick Python trilinos_install_from_scratch script. Or Docker.

@nschloe
Copy link
Contributor Author

nschloe commented Aug 26, 2015

Now we will just wait and see if we get any takers.

This is now two weeks old. Has there been any feedback yet?

@bartlettroscoe
Copy link
Member

This is now two weeks old. Has there been any feedback yet?
Not a single response. I guess that means that if we break someone's default find they can't complain much, right?

When I get some time, I will do some simple testing against CASL VERA and then push. Since we don't use the default find, that testing will not be very hard. Note that I don't find the documentation for the default FindBLAS.cmake and FindLAPACK.cmake modules very good, and I can't figure out, for example, how to get them to point to the Intel MKL. I am sure they are better than what was there but it is hard to tell from the documentation.

@nschloe
Copy link
Contributor Author

nschloe commented Aug 27, 2015

When I get some time, I will do some simple testing against CASL VERA and then push.

Sounds good.

I am sure they are better than what was there but it is hard to tell from the documentation.

Usually it's all about taking a peek in the respective FIND file. For BLAS, for example, I see the environment variable BLAS_VENDOR used.
I totally agree that they aren't good at documenting this.

@nschloe
Copy link
Contributor Author

nschloe commented Sep 18, 2015

So, how about this?

@bartlettroscoe
Copy link
Member

So, how about this?

I will try to get to this in the next few days and push to TriBITS, snapshot to Trilinos, etc.

@bartlettroscoe
Copy link
Member

I am currently locked out of pulling or pushing to the Trilinos git repo on software.sandia.gov. Something going on with my Sandia account.

BTW, the target date for a cleaned up and partitioned Trilinos git repo appropriate to be moved to GitHub has been set for October, 15. That is when people are supposed to be able to inspect the candidate repos. The final repos would be created and transitioned later, hopefully around TUG 2015.

@nschloe
Copy link
Contributor Author

nschloe commented Oct 4, 2015

Anything I can do to help getting this through?

@bartlettroscoe
Copy link
Member

Anything I can do to help getting this through?

It is still near the top of my todo list. It is just that things keep getting dumped on me. Hopefully in the next few days.

Actually, we really need to improve the generic documentation for TPLs in the TriBITS Build Reference. What is there now is pretty sparse and needs to be more useful/helpful. And we might even need to consider having a specialized section for customized TPLs somewhere in that document or in the Trilinos-specific Build Reference.

@bartlettroscoe
Copy link
Member

I pushed the commits:

*   f1c8bf2 "Merge branch 'better-find-blas-lapack-69'" <[email protected]> [Wed Oct 21 22:26:38 2015 -0400] (46 minutes ago)
|\  
| * 971f83d "correct blas/lapack lib names" <[email protected]> [Sun May 31 11:13:30 2015 +0200] (48 minutes ago)
| * 02dfaf2 "update BLAS/LAPACK finders" <[email protected]> [Sun May 31 11:07:56 2015 +0200] (48 minutes ago)
| * c4b31fb "use FIND_PACKAGE(LAPACK)" <[email protected]> [Tue May 19 19:29:34 2015 +0200] (48 minutes ago)
| * a3ed2db "use FIND_PACKAGE(BLAS)" <[email protected]> [Tue May 19 19:29:24 2015 +0200] (48 minutes ago)
| * 7bcc1f9 "typo" <[email protected]> [Tue May 19 19:00:21 2015 +0200] (48 minutes ago)
|/  

This was rebased on top of 'master' so that we get rid of merge commits from 'mater' into the branch and we get a nice looking history, as shown above. This is the workflow step clean up completed topic branch.

I am now in the process of pushing the snapshot to Trilinos proper.

@nschloe
Copy link
Contributor Author

nschloe commented Oct 22, 2015

Aha, so you prefer a rebase over a merge?

@bartlettroscoe
Copy link
Member

Aha, so you prefer a rebase over a merge?

It creates much cleaner history buy avoiding a bunch of merge commits from the main development branch into the topic branch. And it is best practice to keep your topic branch up to date with the main development branch from an Agile CI perspective.

@bartlettroscoe
Copy link
Member

bartlettroscoe commented Oct 22, 2015

This has now been snapshotted to Trilinos in the snapshot commit:

commit f7a23a4814198fcc7e4ca0fd50730d001e33a218
Author: Roscoe A. Bartlett <[email protected]>
Date:   Wed Oct 21 22:26:58 2015 -0400

    Automatic snapshot commit from tribits at f1c8bf2

    Origin repo remote tracking branch: 'origin/master'
    Origin repo remote repo URL: 'origin = [email protected]:TriBITSPub/TriBITS'

    At commit:

    commit 971f83d3acb35cc84ee6701d1816b710af048594
    Author:  Nico Schlömer <[email protected]>
    Date:    Sun May 31 11:13:30 2015 +0200
    Summary: correct blas/lapack lib names

M       cmake/tribits/common_tpls/FindTPLBLAS.cmake
M       cmake/tribits/common_tpls/FindTPLHDF5.cmake
M       cmake/tribits/common_tpls/FindTPLLAPACK.cmake

and I just pushed this.

I tested both the default finds that use FindBLAS.cmake and FindLAPACK.cmake and the TriBITS override where you pass in the exact list of libs.

I am closing this pull request.

@bartlettroscoe
Copy link
Member

Really closing this time.

@nschloe
Copy link
Contributor Author

nschloe commented Oct 22, 2015

It creates much cleaner history buy avoiding a bunch of merge commits from the main development
branch into the topic branch.

I agree, I also prefer rebases actually. Some projects prefer merges, the argument is typically that the existence of merges better reflects what actually happened in reality.

Anyways, thanks for finishing this up! :)

@bartlettroscoe
Copy link
Member

I agree, I also prefer rebases actually. Some projects prefer merges, the argument is typically that the existence of merges better reflects what actually happened in reality.

Actually, the topic branch workflow being advocated is to rebase the topic branch and then merge it --no-ff into the main dev branch. That creates an explicit merge commit. That allows you to back out all of the change by just reverting the one merge commit. If you are interested, take a look at Addition of topic branches. I am in the process of converting this into an ORNL tech report then a mini-book.

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.

2 participants