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

#2448 if HAVE_EPETRA_LAPACK_GSSVD3 then add argument LWORK to Epetra_… #1

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

edmondac
Copy link

…LAPACK_GGSVD_double and Epetra_LAPACK_GGSVD_float

I'm not very familiar with the CTrliinos codebase, so this is my best guess at a fix...

…LAPACK_GGSVD_double and Epetra_LAPACK_GGSVD_float
@mhoemmen
Copy link

mhoemmen commented Apr 1, 2018

@jwillenbring @bartlettroscoe I'm not sure what to do with CTrilinos commits so I'm poking y'all :-)

@edmondac Thanks for contributing! Your comment here trilinos/Trilinos#2448 (comment) says that "I've got a patch that builds on my system, but it wouldn't work in the opposite situation as the header doesn't use an ifdef." What does "wouldn't work in the opposite situation" mean?

@edmondac
Copy link
Author

edmondac commented Apr 1, 2018

Hi @mhoemmen, in my original patch I hadn't included the #ifdef HAVE_EPETRA_LAPACK_GSSVD3 in src/epetra/CEpetra_LAPACK.h - since it wouldn't compile (HAVE_EPETRA_LAPACK_GSSVD3 wasn't defined so I just hardcoded it as if it was). However, in the pull request I think I've fixed that by including +#include "Epetra_config.h" in src/epetra/CEpetra_LAPACK.cpp. So the original patch wouldn't work with older LAPACK, but I hope this PR will...

@mhoemmen
Copy link

mhoemmen commented Apr 1, 2018

@edmondac Including Epetra_config.h should be fine, since CTrilinos has a required package dependency on Epetra. Have you tested this new PR on your computer?

@edmondac
Copy link
Author

edmondac commented Apr 2, 2018

Yep - built like this - with EasyBuild dependency (iomkl/2018a) and Boost/1.65.0-iomkl-2018a-Python-2.7.14 :

cmake -DTPL_ENABLE_MPI=ON -DMPI_BASE_DIR=${EBROOTOPENMPI} -DTrilinos_ENABLE_ALL_PACKAGES=ON -DCMAKE_INSTALL_PREFIX=../MY_INSTALL -DTPL_ENABLE_BLAS:BOOL=ON -DBLAS_LIBRARY_DIRS="${EBROOTIMKL}/mkl/lib/intel64" -DBLAS_LIBRARY_NAMES="mkl_intel_lp64;mkl_intel_thread;mkl_core;iomp5;pthread" -DTPL_ENABLE_LAPACK:BOOL=ON -DLAPACK_LIBRARY_DIRS="${EBROOTIMKL}/mkl/lib/intel64" -DLAPACK_LIBRARY_NAMES="mkl_intel_lp64;mkl_intel_thread;mkl_core;iomp5;pthread" -DTPL_ENABLE_Boost:BOOL=ON -DBoost_INCLUDE_DIRS:PATH="${EBROOTBOOST}/include" -DBoost_LIBRARY_DIRS:PATH="${EBROOTBOOST}/lib" -DTPL_ENABLE_Netcdf=OFF -DTPL_ENABLE_BoostLib=OFF -DTrilinos_ENABLE_MOOCHO:STRING=OFF -DTrilinos_ENABLE_Sundance:STRING=OFF .. && make -j 16

@mhoemmen
Copy link

mhoemmen commented Apr 2, 2018

@edmondac Thanks for the report! I just approved your changes, but I can't merge them myself since I don't have write access to this repository. I think folks like @jwillenbring or @bartlettroscoe might.

@bartlettroscoe
Copy link
Member

Thanks for the report! I just approved your changes, but I can't merge them myself since I don't have write access to this repository. I think folks like @jwillenbring or @bartlettroscoe might.

I don't know that I have the authority to merge PRs with CTrilinos.

@maherou and @jwillenbring,

Who is the Trilinos Product leader for CTrilinos? What Product Area does CTrilinos fall under?

@boegel
Copy link

boegel commented Aug 24, 2018

Any updates on this? I'm hitting the same problem as @edmondac reported in trilinos/Trilinos#2448

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.

4 participants