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

SEACAS Stray character prevents compilation #100

Closed
crtrott opened this issue Jan 25, 2016 · 11 comments
Closed

SEACAS Stray character prevents compilation #100

crtrott opened this issue Jan 25, 2016 · 11 comments
Labels
type: bug The primary issue is a bug in Trilinos code or tests

Comments

@crtrott
Copy link
Member

crtrott commented Jan 25, 2016

Hi

after the current update of seacas I get the following errors:

/home/crtrott/Trilinos/packages/seacas/libraries/suplib/adler.c:1:1: error: expected identifier or \u2018(\u2019 before \u2018/\u2019 token
/home/crtrott/Trilinos/packages/seacas/libraries/suplib/adler.c:8:32: warning: character constant too long for its type [enabled by default]
/home/crtrott/Trilinos/packages/seacas/libraries/suplib/adler.c:16:65: error: unknown type name \u2018you\u2019
/home/crtrott/Trilinos/packages/seacas/libraries/suplib/adler.c:16:74: error: expected \u2018=\u2019, \u2018,\u2019, \u2018;\u2019, \u2018asm\u2019 or \u2018__attribute__\u2019 before \u2018not\u2019
/home/crtrott/Trilinos/packages/seacas/libraries/suplib/adler.c:25:1: error: stray \u2018@\u2019 in program
/home/crtrott/Trilinos/packages/seacas/libraries/suplib/adler.c:25:1: error: stray \u2018@\u2019 in program
In file included from /home/crtrott/Trilinos/packages/seacas/libraries/suplib/adler.c:27:0:
/home/crtrott/Trilinos/packages/seacas/libraries/suplib/adler.h:3:40: error: invalid suffix "AL85000" on integer constant
/home/crtrott/Trilinos/packages/seacas/libraries/suplib/adler.h:27:61: error: unknown type name \u2018LOSS\u2019
/home/crtrott/Trilinos/packages/seacas/libraries/suplib/adler.h:27:69: error: expected \u2018=\u2019, \u2018,\u2019, \u2018;\u2019, \u2018asm\u2019 or \u2018__attribute__\u2019 before \u2018USE\u2019
/home/crtrott/Trilinos/packages/seacas/libraries/suplib/adler.h:28:22: error: unknown type name \u2018OR\u2019
/home/crtrott/Trilinos/packages/seacas/libraries/suplib/adler.h:28:34: error: expected \u2018=\u2019, \u2018,\u2019, \u2018;\u2019, \u2018asm\u2019 or \u2018__attribute__\u2019 before \u2018INTERRUPTION\u2019
/home/crtrott/Trilinos/packages/seacas/libraries/suplib/adler.c:28:41: warning: extra tokens at end of #include directive [enabled by default]
/home/crtrott/Trilinos/packages/seacas/libraries/suplib/adler.c: In function \u2018adler\u2019:
/home/crtrott/Trilinos/packages/seacas/libraries/suplib/adler.c:37:21: warning: declaration of \u2018adler\u2019 shadows a global declaration [-Wshadow]
/home/crtrott/Trilinos/packages/seacas/libraries/suplib/adler.c:37:8: warning: shadowed declaration is here [-Wshadow]

Deleting the license header from both

/home/crtrott/Trilinos/packages/seacas/libraries/suplib/adler.c
/home/crtrott/Trilinos/packages/seacas/libraries/suplib/adler.h

Fixes that, so I guess its just an issue with a windows character or so coming in.

@trilinos/seacas

@crtrott crtrott added the type: bug The primary issue is a bug in Trilinos code or tests label Jan 25, 2016
@gsjaardema
Copy link
Contributor

Thanks, I will fix.

@crtrott
Copy link
Member Author

crtrott commented Jan 25, 2016

Great, thanks!

@gsjaardema
Copy link
Contributor

What compiler were you using for this? It looks like it is getting confused by the C++ comments instead of C /* */ comments...

@crtrott
Copy link
Member Author

crtrott commented Jan 25, 2016

Just gcc 4.7.2

@gsjaardema
Copy link
Contributor

The issue is that the file contained c++ comments '//' instead of c comments '/* */' and the compiler was using the -std=c90 option which seems to ignore the c++ comments which means it was treating the text following the comments as valid code which doesn't work very well for copyright notices and most other comment text. Comment characters have been converted to c format and comitted to Sierra repository. Will be synced to Trilinos soon.

@crtrott
Copy link
Member Author

crtrott commented Jan 25, 2016

Oh ok I misunderstood the error message (and I rarely compile any C code to run into this ...). Thanks for identifying and fixing.

@bmpersc
Copy link
Contributor

bmpersc commented Jan 25, 2016

I will make every effort I can to get another Sierra to Trilinos sync done today. I'm currently in the middle of the integration though so doing so right now isn't ideal.

@crtrott
Copy link
Member Author

crtrott commented Jan 25, 2016

Shall I push a short term fix into Trilinos so that our downstream tests like albany and Nalu don't break in the nightlies? Its simple enough to do?

@gsjaardema
Copy link
Contributor

You will have to clear that with @bmpersc ; I'm not sure what that does to his syncing process.

@crtrott
Copy link
Member Author

crtrott commented Jan 25, 2016

I did talk to him, and he is trying to get a sync done today.

@crtrott
Copy link
Member Author

crtrott commented Feb 2, 2016

This is fixed closing.

@crtrott crtrott closed this as completed Feb 2, 2016
jjellio added a commit to jjellio/Trilinos that referenced this issue Mar 2, 2020
Fixes trilinos#6937
Addresses CDOFA trilinos#100

This is an issue to track and attach a PR for the removal of `LD_PRELOAD` from the `jsrun_wrapper`

The original implementation of the `jsrun_wrapper` did not have any `LD_PRELOAD`. The variable was added to enable unit tests to succeed when they allocated memory before calling `MPI_Init`. (see trilinos#6724 ).  When codes allocate memory, the Cuda Hooks will interact with libpami (so MPI  knows where the memory lives).  This requires symbols to be loaded during `MPI_Init` (namely libpami).

When a code allocates before `MPI_Init`, they will get an obscure error: (perhaps others)
```
CUDA Hook Library: Failed to find symbol mem_find_dreg_entries.
```

This is because the cuda hook library assumes MPI has been initialized.

By adding an `LD_PRELOAD` to the wrapper, this allows memory allocations to find the needed symbols, but we are clearly outside the way Spectrum intended the system to be used. This shouldn't block ATDM apps, as they have been functioning up to this point.  This may cause a headache for some unit tests, but it would be good to make sure developers are aware that most cuda-aware MPI implementations tack on extra rules regarding initializing the library.++ (see note below)

We decided to leave the `LD_PRELOAD` in place until other testing issues were ironed out.  This issue and PR should be blocked until ATDM devops decides they are ready to move forward. By removing LD_PRELOAD several unit tests will crash with errors about missing symbols.  The fix will be to ensure that `MPI_Init` is called before `Kokkos::initialize`, which can be easier said than done.

++ Note: this pattern of requiring `MPI_Init` first is not new.
E.g., OpenMPI w/UCX caused these obscure bugs that arose from allocs before initialize.
trilinos#5033 (comment)

@bartlettroscoe
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug The primary issue is a bug in Trilinos code or tests
Projects
None yet
Development

No branches or pull requests

3 participants