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

Build failure with superlu_dist #103

Closed
balay opened this issue Jan 26, 2016 · 16 comments
Closed

Build failure with superlu_dist #103

balay opened this issue Jan 26, 2016 · 16 comments
Assignees
Labels
CLOSED_DUE_TO_INACTIVITY Issue or PR has been closed by the GitHub Actions bot due to inactivity. impacting: configure or build The issue is primarily related to configuring or building MARKED_FOR_CLOSURE Issue or PR is marked for auto-closure by the GitHub Actions bot. pkg: Amesos2

Comments

@balay
Copy link
Contributor

balay commented Jan 26, 2016

We are seeing trilinos build failure [osx] when built with superlu_dist. It appears that packages/amesos2/src/Amesos2_Superludist_TypeMap.hpp is attempting to use both superlu_ddefs.h and superlu_zdefs.h at the same time - causing errors. My understanding is - only one of them should be used - but not both. cc:ing Sherry for clarification.

builderror.txt

@[email protected]
@[email protected]
@[email protected]

@etphipp
Copy link
Contributor

etphipp commented Jan 26, 2016

I've run into problems like this before as well. If I recall correctly, the issue is ensuring you have all of the libraries correct for proper linking of superlu_dist. A configure test tries to compile and link and a short test code to determine which version of superlu_dist you have. If the link fails because the libraries aren't right, it will be confused as to which version you have (probably the configure logic should be a little smarter about this).

I think I diagnosed this by looking at either CMakeFiles/CMakeError.log or CMakeFiles/CMakeOutput.log (I forget which) in your build directory. I think I added a line to Trilinos/cmake/TPLs/FindTPLSuperLUDist.cmake to make the configure stop after the configure test (otherwise it will keep going and overwrite the output from the test with later test output).

-Eric

From: Satish Balay <[email protected]mailto:[email protected]>
Reply-To: trilinos/Trilinos <[email protected]mailto:[email protected]>
Date: Tuesday, January 26, 2016 at 2:26 PM
To: trilinos/Trilinos <[email protected]mailto:[email protected]>
Subject: [EXTERNAL] [Trilinos] Build failure with superlu_dist (#103)

We are seeing trilinos build failure [osx] when built with superlu_dist It appears that packages/amesos2/src/Amesos2_Superludist_TypeMaphpp is attempting to use both superlu_ddefsh and superlu_zdefsh at the same time - causing errors My understanding is - only one of them should be used - but not both cc:ing Sherry for clarification

builderrortxthttps://githubcom/trilinos/Trilinos/files/105697/builderrortxt

@bsmithhttps://github.com/bsmith@mcsanlgov
@sarichhttps://github.com/sarich@mcsanlgov
@xslihttps://github.com/xsli@lblgov

Reply to this email directly or view it on GitHubhttps://github.com//issues/103.

@balay
Copy link
Contributor Author

balay commented Jan 26, 2016

Hm - superlu_dist - by default builds both 'real' and 'complex' interfaces. From SRC/Makefile

all:  double complex16

So the trilinos/TEUCHOS cmake test checks and always enables the flag 'HAVE_TEUCHOS_COMPLEX'?

Is this something we should set on cmake command line to enable or disable complex support? [or disable this test?]

-DHAVE_TEUCHOS_COMPLEX:BOOL=OFF

Also the code attempting to use both includes - where it should use one or the other?

#undef __SUPERLU_SUPERMATRIX
#include "superlu_defs.h"

  namespace D {
#include "superlu_ddefs.h"      // double-precision real definitions    
  }

#ifdef HAVE_TEUCHOS_COMPLEX
  namespace Z {
#include "superlu_zdefs.h"     // double-precision complex definitions  
  }
#endif  // HAVE_TEUCHOS_COMPLEX                                         

@balay
Copy link
Contributor Author

balay commented Jan 27, 2016

Well this broken build was with default OSX compilers.

balay@ipro^~ $ gcc --version
Configured with: --prefix=/Applications/Xcode.app/Contents/Developer/usr --with-gxx-include-dir=/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.11.sdk/usr/include/c++/4.2.1
Apple LLVM version 7.0.2 (clang-700.1.81)
Target: x86_64-apple-darwin15.2.0
Thread model: posix

However if I build with gcc-5 [from brew] - the build works.

balay@ipro^~ $ gcc-5 --version
gcc-5 (Homebrew gcc 5.3.0) 5.3.0
Copyright (C) 2015 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

Is this supposed to work with clang? Perhaps 'namespace' behaves differently with it? [and there should be check for it somewhere?]

@BarrySmith

@mhoemmen
Copy link
Contributor

Amesos2 requires Tpetra, and Tpetra requires C++11. That version of the default OS X compiler uses the GCC 4.2.1 headers (scroll to the right of the "Configured with..." line), which are not new enough.

@balay
Copy link
Contributor Author

balay commented Jan 27, 2016

Hm - trilinos works - if superlu/dist is not enabled. [presumably Tpetra is also build here]
[even elemental solver package - which requires C++11 works with these compilers]

And I suspect Apple will never move away from GCC 4.2.1 headers.

If this won't work - then trilinos cmake should check - and error out with a proper message?

On Wed, 27 Jan 2016, Mark Hoemmen wrote:

Amesos2 requires Tpetra, and Tpetra requires C++11. That version of the default OS X compiler uses the GCC 4.2.1 headers (scroll to the right of the "Configured with..." line), which are not new enough.


Reply to this email directly or view it on GitHub:
#103 (comment)

@balay
Copy link
Contributor Author

balay commented Jan 27, 2016 via email

@mhoemmen
Copy link
Contributor

Default compiler on my Mac laptop is Clang 3.5. It works fine with C++11 and builds Trilinos just fine.

mfh

On Jan 27, 2016, at 10:51 PM, Satish Balay <[email protected]mailto:[email protected]> wrote:

Hm - trilinos works - if superlu/dist is not enabled. [presumably Tpetra is also build here]
[even elemental solver package - which requires C++11 works with these compilers]

And I suspect Apple will never move away from GCC 4.2.1 headers.

If this won't work - then trilinos cmake should check - and error out with a proper message?

On Wed, 27 Jan 2016, Mark Hoemmen wrote:

Amesos2 requires Tpetra, and Tpetra requires C++11. That version of the default OS X compiler uses the GCC 4.2.1 headers (scroll to the right of the "Configured with..." line), which are not new enough.


Reply to this email directly or view it on GitHub:
#103 (comment)

Reply to this email directly or view it on GitHubhttps://github.com//issues/103#issuecomment-175665440.

@bmpersc
Copy link
Contributor

bmpersc commented Jan 27, 2016

The default mac compilers also include clang and clang++. If you switch your configure to use those instead of the gnu wrappers I suspect you will be able to get a successful build. I have been doing this recently on my mac, though I don't recall if I had packages that require C++11 enabled.

@balay
Copy link
Contributor Author

balay commented Jan 27, 2016

To clarify:

On OSX [starting with Xcode-5] - /usr/bin/gcc and /usr/bin/clang are the same 'clang' compiler [even though there are separate binaries].

This issue comes up with trilinos build with superlu_dist enabled with clang. A standalone trilinos build does not have this issue.

Note: I get the same error with clang on OSX aswell as Linux. Builds with gcc on both OSX and linux complete without errors.

The primary problem is with Superlu_dist - where one cannot use both superlu_ddefs.h and superlu_zdefs.h in the same source file.

[So the primary fix should perhaps be in superlu/superlu_dist?]

Trilinos [Amesos2_Superludist_TypeMap.hpp] tries to workarround this issue by using 'namespace' - and attempts to include both include files. I do not know if this is a valid workarround or buggy code.

g++ gives warnings with this usage [but chuggs along]. However clang gives errors.

Presumably the following is a test code demonstrating this usage of including superlu_ddefs.h and superlu_zdefs.h in Amesos2_Superludist_TypeMap.hpp

balay@asterix /home/balay/junk
$ g++ --version
g++ (GCC) 5.3.1 20151207 (Red Hat 5.3.1-2)
Copyright (C) 2015 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

balay@asterix /home/balay/junk
$ clang++ --version
clang version 3.7.0 (tags/RELEASE_370/final)
Target: x86_64-redhat-linux-gnu
Thread model: posix
balay@asterix /home/balay/junk
$ cat ns.cxx
extern "C" {
  namespace X {
    extern void foo(int);
  }
  namespace Y {
    extern void foo(double);
  }
}

int main(void)
{
  return 0;
}
balay@asterix /home/balay/junk
$ g++ ns.cxx
ns.cxx:6:27: warning: declaration of ‘void Y::foo(double)’ with C language linkage
     extern void foo(double);
                           ^
ns.cxx:3:17: warning: conflicts with previous declaration ‘void X::foo(int)’
     extern void foo(int);
                 ^
balay@asterix /home/balay/junk
$ clang++ ns.cxx
ns.cxx:6:17: error: conflicting types for 'foo'
    extern void foo(double);
                ^
ns.cxx:3:17: note: previous declaration is here
    extern void foo(int);
                ^
1 error generated.
balay@asterix /home/balay/junk
$ 

On Wed, 27 Jan 2016, Brent Perschbacher wrote:

The default mac compilers also include clang and clang++. If you switch your configure to use those instead of the gnu wrappers I suspect you will be able to get a successful build. I have been doing this recently on my mac, though I don't recall if I had packages that require C++11 enabled.


Reply to this email directly or view it on GitHub:
#103 (comment)

@balay
Copy link
Contributor Author

balay commented Jan 27, 2016

Bah - formatting from e-mail is messed up - so refiling the info using the web interface

To clarify:

On OSX [starting with Xcode-5] - /usr/bin/gcc and /usr/bin/clang are the same 'clang' compiler [even though there are separate binaries].

This issue comes up with trilinos build with superlu_dist enabled with clang. A standalone trilinos build does not have this issue.

Note: I get the same error with clang on OSX aswell as Linux. Builds with gcc on both OSX and linux complete without errors.

The primary problem is with Superlu_dist - where one cannot use both superlu_ddefs.h and superlu_zdefs.h in the same source file.

[So the primary fix should perhaps be in superlu/superlu_dist?]

Trilinos [Amesos2_Superludist_TypeMap.hpp] tries to workarround this issue by using 'namespace' - and attempts to include both include files. I do not know if this is a valid
workarround or buggy code.

g++ gives warnings with this usage [but chuggs along]. However clang gives errors.

Presumably the following is a test code demonstrating this usage of including superlu_ddefs.h and superlu_zdefs.h in Amesos2_Superludist_TypeMap.hpp

balay@asterix /home/balay/junk
$ g++ --version
g++ (GCC) 5.3.1 20151207 (Red Hat 5.3.1-2)
Copyright (C) 2015 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

balay@asterix /home/balay/junk
$ clang++ --version
clang version 3.7.0 (tags/RELEASE_370/final)
Target: x86_64-redhat-linux-gnu
Thread model: posix
balay@asterix /home/balay/junk
$ cat ns.cxx
extern "C" {
  namespace X {
    extern void foo(int);
  }
  namespace Y {
    extern void foo(double);
  }
}

int main(void)
{
  return 0;
}
balay@asterix /home/balay/junk
$ g++ ns.cxx
ns.cxx:6:27: warning: declaration of ‘void Y::foo(double)’ with C language linkage
     extern void foo(double);
                           ^
ns.cxx:3:17: warning: conflicts with previous declaration ‘void X::foo(int)’
     extern void foo(int);
                 ^
balay@asterix /home/balay/junk
$ clang++ ns.cxx
ns.cxx:6:17: error: conflicting types for 'foo'
    extern void foo(double);
                ^
ns.cxx:3:17: note: previous declaration is here
    extern void foo(int);
                ^
1 error generated.
balay@asterix /home/balay/junk
$ 

@srajama1
Copy link
Contributor

Balay : Amesos2 supports double and complex version of SuperLU-Dist but it cannot do that it in the same build (because of conflicts in SuperLU-Dist).

As you correctly observed setting -DHAVE_TEUCHOS_COMPLEX:BOOL=OFF if you don't need complex.

@mhoemmen : Any ideas why Complex is ON by default ? Can we turn it off ?

If you are ok with that option, I will close this issue. If you require support for both complex and double in the same build, then we need to request a change from SuperLU-Dist.

@mhoemmen
Copy link
Contributor

Any ideas why Complex is ON by default ? Can we turn it off ?

There's a difference between "complex enabled in Teuchos" and "complex enabled in Tpetra and downstream packages." It is possible to enable complex in Teuchos, but not enable complex in Tpetra. The Teuchos option costs much less (build time, library and executable sizes, and tests' run time) than the Tpetra option. Furthermore, users get annoyed when they have to set some Teuchos option in order to get complex arithmetic in Tpetra.

@srajama1
Copy link
Contributor

This is positively confusing even for developers even more confusing for users. Can we have a Trilinos_ENABLE_Complex and everything else derive from it ?

Amesos2 uses Teuchos because that is what was standard when Amesos2 was written. If Tpetra depends on something else should I use that too ? What if they conflict ? One option will get rid of all these confusion.

@srajama1 srajama1 self-assigned this Feb 20, 2016
@mhoemmen
Copy link
Contributor

On Feb 20, 2016, at 4:42 PM, Siva Rajamanickam <[email protected]mailto:[email protected]> wrote:

Can we have a Trilinos_ENABLE_Complex and everything else derive from it ?

Teuchos_ENABLE_Complex should really be ON by default. In fact, it should not even exist as an option. The main effect for solver developers is that Teuchos::ScalarTraits works for std::complex, which it should do by default anyway.

Would you mind filing an issue and CC'ing Ross and me? Ross might know whether enabling this by default could introduce some unforeseen complication, in particular for Thyra.

Amesos2 uses Teuchos because that is what was standard when Amesos2 was written. If Tpetra depends on something else should I use that too ? What if they conflict ? One option will get rid of all these confusion.

Tpetra's option for complex relates to ETI and the set of enabled Scalar types. Enabling that by default adds at least one Scalar type to the build.

Tpetra's CMake logic explicitly forbids enabling complex if Teuchos_ENABLE_Complex is OFF.

@jhux2 jhux2 added the impacting: configure or build The issue is primarily related to configuring or building label Mar 3, 2016
@github-actions
Copy link

This issue has had no activity for 365 days and is marked for closure. It will be closed after an additional 30 days of inactivity.
If you would like to keep this issue open please add a comment and remove the MARKED_FOR_CLOSURE label.
If this issue should be kept open even with no activity beyond the time limits you can add the label DO_NOT_AUTOCLOSE.

@github-actions github-actions bot added the MARKED_FOR_CLOSURE Issue or PR is marked for auto-closure by the GitHub Actions bot. label Dec 15, 2020
@github-actions
Copy link

This issue was closed due to inactivity for 395 days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLOSED_DUE_TO_INACTIVITY Issue or PR has been closed by the GitHub Actions bot due to inactivity. impacting: configure or build The issue is primarily related to configuring or building MARKED_FOR_CLOSURE Issue or PR is marked for auto-closure by the GitHub Actions bot. pkg: Amesos2
Projects
None yet
Development

No branches or pull requests

6 participants