neuronsimulator / nrn

NEURON Simulator
http://nrn.readthedocs.io
Other
406 stars 118 forks source link

NRN_ENABLE_MUSIC on wheels #2091

Open nrnhines opened 2 years ago

nrnhines commented 2 years ago

This is intended for implementation discussion toward a neuron wheel which works in the absence of a MUSIC installation but is then able to use MUSIC after it is installed. Presently MUSIC + NEURON works with build time linking ( PR #1896 relating to issue #1894 ).

See PR #2092

nrnhines commented 2 years ago

On a Mac with MUSIC installed

export PATH=$HOME/neuron/MUSIC/musicinstall/bin:$PATH
cmake .. -G Ninja -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ -DCMAKE_INSTALL_PREFIX=install -DPYTHON_EXECUTABLE=`which python3.11` -DNRN_ENABLE_RX3D=OFF -DCMAKE_BUILD_TYPE=Debug -DNRN_CLANG_FORMAT=ON -DNRN_ENABLE_TESTS=ON -DNRN_ENABLE_MUSIC=ON -DCMAKE_PREFIX_PATH=$HOME/neuron/MUSIC/musicinstall -DNRN_ENABLE_MPI_DYNAMIC=ON
ninja install
ctest -R nrnmusic::music_tests

The last command fails 62 - nrnmusic::music_tests (Failed) But we are at the point where dynamic loading of music occurs without error.

% nrniv -music
nrnmusic_load
NEURON -- VERSION 9.0.dev-152-g7cf0bfd04+ hines/music-dynamic (7cf0bfd04+) 2022-11-16
Duke, Yale, and the BlueBrain Project -- Copyright 1984-2022
See http://neuron.yale.edu/neuron/credits

oc>

Note:

hines@michaels-macbook-pro music_tests % pwd
/Users/hines/neuron/nrnmusic/test/music_tests
hines@michaels-macbook-pro music_tests % mpirun -np 2 music test2.music
NEURON -- VERSION 9.0.dev-152-g7cf0bfd04+ hines/music-dynamic (7cf0bfd04+) 2022-11-16
Duke, Yale, and the BlueBrain Project -- Copyright 1984-2022
See http://neuron.yale.edu/neuron/credits

music: syntax error
 in test2.music near line 2
 [neuron]
 ^
hoc_run1: caught exception: hoc_execerror
--------------------------------------------------------------------------
Primary job  terminated normally, but 1 process returned
a non-zero exit code. Per user-direction, the job has been aborted.

Is there a way to launch nrniv -music from the terminal and interact with music?

nrnhines commented 2 years ago

Presently with the linked build I see:

hines@michaels-macbook-pro music_tests % nrniv -music -python test2.py
numprocs=1
NEURON -- VERSION 9.0.dev-153-gf8cd40063+ hines/music-dynamic (f8cd40063+) 2022-11-17
Duke, Yale, and the BlueBrain Project -- Copyright 1984-2022
See http://neuron.yale.edu/neuron/credits

Error in MUSIC library: attempt to map port `out' which is unconnected

which make sense since music was not launched as the driver. But with the dynamic build I see:

hines@michaels-macbook-pro music_tests % nrnenv nrnmusic/builddynam/install
hines@michaels-macbook-pro music_tests % lldb nrniv
(lldb) target create "nrniv"
Current executable set to 'nrniv' (arm64).
(lldb) run -music -python test2.py
Process 78194 launched: '/Users/hines/neuron/nrnmusic/builddynam/install/bin/nrniv' (arm64)
nrnmusic_load
NEURON -- VERSION 9.0.dev-153-gf8cd40063+ hines/music-dynamic (f8cd40063+) 2022-11-17
Duke, Yale, and the BlueBrain Project -- Copyright 1984-2022
See http://neuron.yale.edu/neuron/credits

Process 78194 stopped
* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x51)
    frame #0: 0x00000001025aa350 libmusic.1.dylib`MUSIC::Setup::maybePostponedSetup() + 16
libmusic.1.dylib`MUSIC::Setup::maybePostponedSetup:
->  0x1025aa350 <+16>: ldrb   w8, [x0, #0x51]
    0x1025aa354 <+20>: cbz    w8, 0x1025aa3d0           ; <+144>
    0x1025aa358 <+24>: mov    x19, x0
    0x1025aa35c <+28>: strb   wzr, [x0, #0x51]
Target 0: (nrniv) stopped.
(lldb) bt
* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x51)
  * frame #0: 0x00000001025aa350 libmusic.1.dylib`MUSIC::Setup::maybePostponedSetup() + 16
    frame #1: 0x00000001025bacd8 libmusic.1.dylib`MUSIC::Port::Port(MUSIC::Setup*, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >) + 76
    frame #2: 0x00000001006509c8 libnrnmusic_ompi.dylib`NRNMUSIC::EventOutputPort::EventOutputPort(this=0x0000600002908000, s=0x0000000000000000, id=<unavailable>) at nrnmusic.h:22:11
    frame #3: 0x00000001006508e8 libnrnmusic_ompi.dylib`NRNMUSIC::publishEventOutput(id="out") at nrnmusic.cpp:133:16
    frame #4: 0x0000000104ab2684 neuronmusic.cpython-311-darwin.so`__pyx_pw_11neuronmusic_1publishEventOutput(_object*, _object*) [inlined] __pyx_pf_11neuronmusic_publishEventOutput(__pyx_self=<unavailable>, __pyx_v_identifier=<unavailable>) at neuronmusic.cpp:2266:57 [opt]
    frame #5: 0x0000000104ab24c0 neuronmusic.cpython-311-darwin.so`__pyx_pw_11neuronmusic_1publishEventOutput(__pyx_self=<unavailable>, __pyx_v_identifier=<unavailable>) at neuronmusic.cpp:2221:13 [opt]
nrnhines commented 2 years ago

Now getting same result for nrniv -music -python test2.py for linked and dynamic versions. I.e.

Error in MUSIC library: attempt to map port `out' which is unconnected

But dynamic version for mpirun -np 2 music test2.music still giving

music: syntax error
 in test2.music near line 2
 [neuron]
 ^
hoc_run1: caught exception: hoc_execerror
nrnhines commented 1 year ago

The problem with mpirun -np 2 music test2.music is that music launches nrniv with music args instead of nrniv args. Ie. when printing args on launch of nrniv and when music setup.cc Setup::fullInit () looks up the configuration args, I see

hines@michaels-macbook-pro music_tests % mpirun -np 2 music test2.music
<following 3 lines printed from nrn/src/ivoc/nrnmain.cpp>
argc=2
argv[0]=|music|
argv[1]=|test2.music|
<following 2 lines printed from MUSIC/src/setup.cc>
Setup::fullInit eventlogger   
Setup::fullInit nrniv   -music -nobanner -python test2.py
numprocs=1

This also explains why the -nobanner arg had no effect in test3.music.

nrnhines commented 1 year ago

I can temporarily work around the problem by checking for a "music" arg as well as a "-music" arg.

mdjurfeldt commented 1 year ago

@nrnhines The proper way to handle it would be to simply always create the MUSIC Setup object when NEURON is compiled with MUSIC. There's a method setup->launchedByMusic () which returns a boolean which can be used to check if this is a MUSIC-controlled simulation if that is needed. Note that the Setup object constructor is equivalent to MPI_Init.

This is a consequence of the MPI standard where an application can't really make any assumptions regarding argc and argv before having called MPI_Init.

I'm open to discussion if you think this is a bad idea and have alternative solutions.

nrnhines commented 1 year ago

@mdjurfeldt Thanks. I remember when I started with MPI that I wracked by brains for a long time trying to figure out how nrniv could know if it had been launched by mpirun (and therefore could call MPI_Init (or MPI_Initialized()?). Hence the invention of the -mpi arg. Even today my understanding of this area is weak. Python certainly has no need for such a hint!

% python3 -c 'from mpi4py.MPI import COMM_WORLD ; print (COMM_WORLD.Get_rank(), COMM_WORLD.Get_size())' 
0 1
% mpirun -n 4 python3 -c 'from mpi4py.MPI import COMM_WORLD ; print (COMM_WORLD.Get_rank(), COMM_WORLD.Get_size())'
0 4
1 4
2 4
3 4

I was unaware of that aspect of the MPI standard and am confused then about how python can know its -c arg before calling MPI_Init

Anyway, the issue for dynamic loading boils down to when is the best time to dynamically load libmpi and the corresponding neuron mpi interface wrapper libnrnmpi_ompi as well as dynamically load libmusic and its corresponding neuron music interface wrapper libnrnmusic_ompi. One possibility is to go ahead at the beginning and try to dynamically load them all some time after launch without bothering with whether the -mpi or -music args are present and without raising an error if they don't exist. There would be an error later on if one tried to use them and they did not exist.

nrnhines commented 1 year ago

I think the dynamic loading of music is now basically sound. All the nrnmusic components have been factored into nrn/src/neuronmusic. CMakeLists.txt, setup.py.in are kind of a mess and can be cleaned up. nrnmusic_dynam.cpp needs to be made robust with respect to the path to libmusic and OS . Right now for my experiments I have it hardcoded with

#define NRN_LIBDIR "/Users/hines/neuron/nrnmusic/builddynam/install/lib/"
void* handle = dlopen(NRN_LIBDIR "libnrnmusic_ompi.dylib", RTLD_NOW | RTLD_LOCAL);

One possibility is to use the last resort strategy used by nrnmpi which checks the environment variable const_char_ptr{std::getenv("MPI_LIB_NRN_PATH"). On my machine that is

export MPI_LIB_NRN_PATH=/opt/homebrew/Cellar/open-mpi/4.1.2/lib/libmpi.dylib

@mdjurfeldt is there some equivalent already to hand if music is installed?

edit: what I left out above, but is done for mpi, is that the full path to libmusic must be dlopen'd first and then examined to determine what mpi it used and then dlopen the compatible version of libnrnmusic_xxxx.

mdjurfeldt commented 1 year ago

@nrnhines With regard to library path, I have no other suggestions than 1) rely on the standard system locations (as setup by ldconfig/LD_LIBRARY_PATH and available via dlopen) or 2) giving the possibility to specify through a cmake option where to look for MUSIC.

With regard to how Python can see "-c" I find that very interesting and will try to examine it and return to you.

nrnhines commented 1 year ago

2) giving the possibility to specify through a cmake option where to look for MUSIC.

Won't work because music may be installed in a different path on the user machine than music was installed on the build machine. Nevertheless, it may not be too far off to first try the environment variable MUSIC_LIB_NRN_PATH and then your 2) standard system locations, and lastly, look in (from the result of which music) /where/music/was/installed/bin/../lib and then, if one tries to access libmusic, raise an error saying libmusic is not loaded. Note that it will be feasible, though we may not wish to do it if all pythons are using openmpi, to supply two versions of the libnrnmusic_ompi and libnrnmusic_mpich.

nrnhines commented 1 year ago

With regard to how Python can see "-c" I find that very interesting and will try to examine it and return to you.

I'd appreciate a fuller understanding. I'm thinking that as a practical matter, mpirun -n 2 prog args goes ahead uses prog args, along with possibly other rank specific arguments, for argc,argv, for each invocation of prog. So it kind of makes sense that when mpirun -n 2 music test2.music launches nrniv (without any args?) it inherits the argc,argv of the music launch.

nrnhines commented 1 year ago

@mdjurfeldt An issue has come up with regard to MUSIC having a GPL licence. This means we can't support a MUSIC enabled wheel version of NEURON. I wonder if just those few files that we need could be changed to a BSD licence. Or perhaps a minimal API file with only those items we reference could be a separate BSD file.

mdjurfeldt commented 1 year ago

@nrnhines I'm sure this is solvable, but first: Could you please say a few words about the reason for this. I thought that BSD and GPL were "compatible".

nrnhines commented 1 year ago

I'm the wrong person to ask. And perhaps an explanation link is in order. But I believe GPL is more restrictive and "infects" all the package of which it is a part to also elevate the entire package licence to GPL.

nrnhines commented 1 year ago

https://en.wikipedia.org/wiki/License_compatibility#:~:text=Many%20of%20the%20most%20common,LGPL%2C%20are%20GPL%2Dcompatible. at least for a sense of the complications

alexsavulescu commented 1 year ago

@mdjurfeldt it is my understanding that using GPL code in NEURON means that NEURON must also become GPL. What are your requirements for having a GPL license?

A couple of solutions:

mdjurfeldt commented 1 year ago

@nrnhines @alexsavulescu I have previously considered shifting to LGPL. I'll prepare a new release (which also contains an important bugfix for the Python interface build system) and tell you. This can happen during the weekend at the earliest since I'm traveling.

pramodk commented 1 year ago

👍

This can happen during the weekend at the earliest since I'm traveling.

Just a random guess - in case you are at the HBP summit, our team members @iomaganaris and @jamesgking are there as well. In case you get a chance to say hello to each other :)

mdjurfeldt commented 1 year ago

@pramodk Thanks, yes I am. I'll keep my eyes open. :)

mdjurfeldt commented 1 year ago

@alexsavulescu Hi, I've discovered a problem. MUSIC parses its configuration files using code based on rudeconfig, which is GPL. So, I'll need to replace the configuration file parser in order to be able to release MUSIC under LGPL. This can be done, but I also have other urgent business to take care of during the coming couple of weeks.

How urgent is it for you to get this fixed? Is there some deadline which we should meet?

alexsavulescu commented 1 year ago

@mdjurfeldt I see, thanks for the heads-up. There is no deadline and we can adjust accordingly, but do note that more work is required in order to have MUSIC enabled wheels. Please see https://github.com/neuronsimulator/nrn/pull/2096#issuecomment-1472128067