Skip to content

Re-enable MUSIC support in NEURON#1896

Merged
nrnhines merged 25 commits into
masterfrom
hines/enable-music
Dec 2, 2022
Merged

Re-enable MUSIC support in NEURON#1896
nrnhines merged 25 commits into
masterfrom
hines/enable-music

Conversation

@ohm314

@ohm314 ohm314 commented Jul 1, 2022 •
edited by nrnhines
Loading

Copy link
Copy Markdown
Member

This relates to #1894
#2092 extends this to dynamic loading when NRN_ENABLE_MPI_DYNAMIC=ON
#2096 extends that to wheels.

MUSIC support was inadvertently dropped when we switched to CMake. We're fixing this oversight now. This is still a bit work in progress but we have added the FindPackage module and most of the CMake piping to get things built. Mostly what's left to do is:

  • Figure out how to deal with deprecated MPI C++ interface
  • Check how this behaves when building a wheel. (see Wheel that supports MUSIC if installed. #2096)
  • Check if OS/table.h can be replaced with a standard data structure
  • Check if we should link against the .so or .a library.

nrnhines
nrnhines reviewed Jul 1, 2022
View reviewed changes
Comment thread cmake/BuildOptionDefaults.cmake Outdated
nrnhines
nrnhines reviewed Jul 1, 2022
View reviewed changes
Comment thread cmake/modules/FindMusic.cmake Outdated
nrnhines
nrnhines reviewed Jul 1, 2022
View reviewed changes
Comment thread docs/cmake_doc/options.rst Outdated
nrnhines
nrnhines reviewed Jul 1, 2022
View reviewed changes
Comment thread CMakeLists.txt Outdated
nrnhines
nrnhines reviewed Jul 1, 2022
View reviewed changes
Comment thread CMakeLists.txt Outdated
@azure-pipelines

azure-pipelines Bot commented Jul 7, 2022

Copy link
Copy Markdown

✔️ a987364b5154cb5230b55eacb0b029c0b29bb448 -> Azure artifacts URL

@azure-pipelines

azure-pipelines Bot commented Aug 24, 2022

Copy link
Copy Markdown

✔️ aee0a29a565d62d72fe9297f68391ac79c1ce326 -> Azure artifacts URL

@nrnhines nrnhines mentioned this pull request Aug 24, 2022
Closed
@ohm314 ohm314 force-pushed the hines/enable-music branch from aee0a29 to 871bc1c Compare August 26, 2022 12:46
@azure-pipelines

azure-pipelines Bot commented Aug 29, 2022

Copy link
Copy Markdown

✔️ 871bc1c -> Azure artifacts URL

@bbpbuildbot

bbpbuildbot commented Nov 11, 2022

Copy link
Copy Markdown
Collaborator

Logfiles from GitLab pipeline #84050 (:no_entry:) have been uploaded here!

Status and direct links:

@bbpbuildbot

bbpbuildbot commented Nov 11, 2022

Copy link
Copy Markdown
Collaborator

Logfiles from GitLab pipeline #84055 (:no_entry:) have been uploaded here!

Status and direct links:

@bbpbuildbot

bbpbuildbot commented Nov 11, 2022

Copy link
Copy Markdown
Collaborator

Logfiles from GitLab pipeline #84056 (:white_check_mark:) have been uploaded here!

Status and direct links:

@bbpbuildbot

bbpbuildbot commented Nov 13, 2022

Copy link
Copy Markdown
Collaborator

Logfiles from GitLab pipeline #84120 (:white_check_mark:) have been uploaded here!

Status and direct links:

@azure-pipelines

azure-pipelines Bot commented Nov 13, 2022

Copy link
Copy Markdown

✔️ 306a39d -> Azure artifacts URL

@nrnhines nrnhines marked this pull request as ready for review November 13, 2022 16:48
@bbpbuildbot

bbpbuildbot commented Nov 13, 2022

Copy link
Copy Markdown
Collaborator

Logfiles from GitLab pipeline #84127 (:no_entry:) have been uploaded here!

Status and direct links:

@azure-pipelines

azure-pipelines Bot commented Nov 13, 2022

Copy link
Copy Markdown

✔️ 0fc8fdd -> Azure artifacts URL

@nrnhines

nrnhines commented Nov 14, 2022

Copy link
Copy Markdown
Member

@mdjurfeldt You mentioned in the README (Now in nrn/test/music_tests/README) that

(For some reason, it is not possible to add -nobanner for test3.)

I don't know details about gnu_getopt but if you skip one more...

-optlist, args = gnu_getopt(argv[3:], "g:")
+optlist, args = gnu_getopt(argv[4:], "g:")

works when

argv= ['nrniv', '-nobanner', '-music', '-python', 'test3.py', '-g', '1']

@nrnhines nrnhines mentioned this pull request Nov 17, 2022
Open
@ohm314 ohm314 closed this Nov 21, 2022
@ohm314 ohm314 reopened this Nov 21, 2022
@bbpbuildbot

bbpbuildbot commented Nov 21, 2022

Copy link
Copy Markdown
Collaborator

Logfiles from GitLab pipeline #85987 (:white_check_mark:) have been uploaded here!

Status and direct links:

@bbpbuildbot

bbpbuildbot commented Nov 21, 2022

Copy link
Copy Markdown
Collaborator

Logfiles from GitLab pipeline #85988 (:white_check_mark:) have been uploaded here!

Status and direct links:

@azure-pipelines

azure-pipelines Bot commented Nov 21, 2022

Copy link
Copy Markdown

✔️ 1d012b5 -> Azure artifacts URL

@bbpbuildbot

bbpbuildbot commented Nov 24, 2022

Copy link
Copy Markdown
Collaborator

Logfiles from GitLab pipeline #86732 (:white_check_mark:) have been uploaded here!

Status and direct links:

@nrnhines nrnhines mentioned this pull request Nov 24, 2022
Draft
@azure-pipelines

azure-pipelines Bot commented Nov 24, 2022

Copy link
Copy Markdown

✔️ f657fa6 -> Azure artifacts URL

@nrnhines

nrnhines commented Dec 2, 2022

Copy link
Copy Markdown
Member

@ohm314 What is your opinion about merging this when it passes CI? I'm working toward a mergable #2096 for wheels, but that could still take a while.

@azure-pipelines

azure-pipelines Bot commented Dec 2, 2022

Copy link
Copy Markdown

✔️ 7b93603 -> Azure artifacts URL

@bbpbuildbot

bbpbuildbot commented Dec 2, 2022

Copy link
Copy Markdown
Collaborator

Logfiles from GitLab pipeline #88313 (:no_entry:) have been uploaded here!

Status and direct links:

@nrnhines nrnhines merged commit 76adc58 into master Dec 2, 2022
alexsavulescu pushed a commit that referenced this pull request Dec 7, 2022
@nrnhines @mdjurfeldt @alexsavulescu
Re-enable MUSIC support in NEURON (#1896)
94f25ac 
* Enable MUSIC with -DNRN_ENABLE_MUSIC=ON

* Improve the FindPackage module for MUSIC

* ctest -R nrnmusic::music_tests

Co-authored-by: Michael Hines <michael.hines@yale.edu>
Co-authored-by: Mikael Djurfeldt <mikael@djurfeldt.com>
@pramodk pramodk mentioned this pull request Dec 15, 2022
Merged
alexsavulescu pushed a commit that referenced this pull request Dec 15, 2022
@nrnhines @mdjurfeldt @alexsavulescu
Re-enable MUSIC support in NEURON (#1896)
7d02ca6 
* Enable MUSIC with -DNRN_ENABLE_MUSIC=ON

* Improve the FindPackage module for MUSIC

* ctest -R nrnmusic::music_tests

Co-authored-by: Michael Hines <michael.hines@yale.edu>
Co-authored-by: Mikael Djurfeldt <mikael@djurfeldt.com>
alexsavulescu pushed a commit that referenced this pull request Dec 15, 2022
@nrnhines @mdjurfeldt @alexsavulescu
Re-enable MUSIC support in NEURON (#1896)
138b127 
* Enable MUSIC with -DNRN_ENABLE_MUSIC=ON

* Improve the FindPackage module for MUSIC

* ctest -R nrnmusic::music_tests

Co-authored-by: Michael Hines <michael.hines@yale.edu>
Co-authored-by: Mikael Djurfeldt <mikael@djurfeldt.com>
@alexsavulescu alexsavulescu mentioned this pull request Dec 15, 2022
Closed
25 tasks
@alexsavulescu alexsavulescu deleted the hines/enable-music branch February 8, 2023 11:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nrnhines nrnhines nrnhines approved these changes
+1 more reviewer
@mdjurfeldt mdjurfeldt mdjurfeldt left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@ohm314 @bbpbuildbot @codecov-commenter @nrnhines @mdjurfeldt