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

Fix 28 mymodule bluegene #55

Merged
merged 4 commits into from Jul 14, 2015
Merged

Conversation

heplesser
Copy link
Contributor

This PR should fix #28 and also partially address trac.526 and trac.617. Specifically

  • DynModule is completely removed, as they no longer differed in a meaningful way from SLIModule
  • The last code bits left of the never fully implemented module unloading mechanism removed
  • When compiling for BlueGene, user modules (based on MyModule) are only built as static libraries and are linked statically into NEST. Other platforms are not affected by this change

Potential reviews: @jougs @tammoippen @apeyser

Hans Ekkehard Plesser and others added 4 commits July 1, 2015 13:45
- There was no real difference left between SLIModule and DynModule, therefore DynModule is removed now.
- All modules are now created as SLIModule(s).
- As a side-effect, the `Uninstall` function for modules has been removed, but it never worked anyways, because DynModule::unregister() only threw an exception; this partially solves trac.617.
- On BlueGene, the DynamicLoaderModule is not intialized any longer. This is important, because user-defined models are linked statically on BlueGene and are loaded and initialized via static_modules.h.
… modules for BlueGene, but it has so far only been tested "fake" on OSX.
@tammoippen
Copy link
Contributor

The file sli/interpreter.cc does not conform to clang-format. Can you call clang-format on this file again.

@tammoippen
Copy link
Contributor

The code looks good to me. +1 for merge after TravisCI is satisfied.

@heplesser
Copy link
Contributor Author

@tammoippen I don't understand the formatting problems. On my machine, I get

[plesser@Hanss-MBP] ~/NEST/code/trunk/src/sli [fix_28_mymodule_bluegene*](0)$ clang-format interpret.cc > interpret.cc.clang
[plesser@Hanss-MBP] ~/NEST/code/trunk/src/sli [fix_28_mymodule_bluegene*](0)$ diff interpret.cc interpret.cc.clang 
[plesser@Hanss-MBP] ~/NEST/code/trunk/src/sli [fix_28_mymodule_bluegene*](0)$ clang-format --version
clang-format version 3.7.0 (tags/google/testing/2015-06-18)

and this line about which clang-format complains on Travis

 out << std::endl << msg << errorname;

seems perfectly fine to me.

I am also surprised that Travis only reports build 89.8 as failing, even though all other builds have the same code.

@tammoippen
Copy link
Contributor

Unfortunately, this is due to the version difference: TravisCI uses clang-format 3.6 and on your Mac you use 3.7 (same with me). Also there is no flag in 3.7, which mimics the 3.6 behaviour.

I am also surprised that Travis only reports build 89.8 as failing, even though all other builds have the same code.

I am also surprised by this: all builds should have failed...

@jougs
Copy link
Contributor

jougs commented Jul 8, 2015

I'm also happy with the code, so +1 for merging. @tammoippen Can you please click the button if you think that the failing Travis is just a glitch in the format checker? Thanks!

@apeyser
Copy link
Contributor

apeyser commented Jul 8, 2015

If this is a glitch, shouldn't this branch get pulled and run against clang-3.6 so that we don't introduce failing files into the stream?

@jougs
Copy link
Contributor

jougs commented Jul 8, 2015

@apeyser Sounds reasonable.

@tammoippen
Copy link
Contributor

It seems TravisCI is using different versions of the build.sh script. In the failing build, the changes from #54 are included, which makes wrong formatted code fail. The other builds do not have these changes, but also report the wrong formatted code as before (but do not fail).

I agree with @apeyser, but do not have write permissions to @heplesser's branch.

@tammoippen tammoippen merged commit c22f2fb into nest:master Jul 14, 2015
tammoippen added a commit that referenced this pull request Jul 14, 2015
@heplesser heplesser deleted the fix_28_mymodule_bluegene branch August 5, 2015 13:55
Helveg pushed a commit to Helveg/nest-simulator that referenced this pull request Aug 25, 2021
Dynamic generation of the nest module and the kernel attributes.
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.

MyModule does not work on BlueGene
4 participants