Allow multiple Setup instances

[uahic]

• This fix allows users to use the python bindings to create ports and get configuration variables even if they are already running a music enabled simulator instance.
• The changes causes a exception (which is suppressed) in the python bindings, e.g.

Exception AttributeError: "'pymusic.ContOutputPort' object has no attribute 'ptr'" in 'pymusic.Setup.dealloc' ignored.

I could not fix it until now but except from a warning when shutting down the application it does not cause problems.

[mdjurfeldt]

@uahic I think your change is fine---many thanks!--- and I will merge it if you fix three things:

1. It is important that calls to the Setup constructor modifies the arguments argc and argv to the values specified by MUSIC configuration information for every call to the Setup constructor, i.e. also when previous Setup instances exist. Otherwise, the order of initialization of PyNEST and PyMUSIC begins to matter.
2. Could you eliminate the code repetition between the two call signatures of the Setup constructor?
3. Please align the indentation style to the style I (attempt) to maintain in MUSIC, e.g. "if( ... )" --> "if (...)".

Best regards, Mikael

[uahic]

Thanks for having a look at it! Sure, give me a day or so and Ill deliver the changes

[uahic]

Modifying the argc and argv on every setup call is somewhat not possible without getting unclean and issues. For instance these arguments are also passed to MPI::Init which is called only once. So the order could still matter. I feel that my solution is not clean from a software engineering perspective

Possibly its better that i add a bit of Cython interface code to return NEST's internal music-setup. What do you think?

[apeyser]

@uahic: I agree that the MPI Comm and the MUSIC Setup should be accessible.

[mdjurfeldt]

@uahic: No, I think your impression of uncleanliness comes from the idea that you pass arguments to MPI_Init. It's the opposite way: Arguments are written to. So, I think your solution, with my suggested changes, is OK.

There is, however, one ordering problem: If one of the applications invokes the threaded version of Setup (like NEST might) while the other invokes the normal one, then there is a conflict. But if this is done, we could throw an error pointing the users towards using another ordering of the initialization.

@uahic @apeyser: I still think it would be nice if NEST MUSIC Setup would be accessible through the Cython interface, so I agree here. What do you two say about the idea to do both (merging this pull request + providing access to NEST Setup object)?

[uahic]

@mdjurfeldt ok, I am looking right now into it; another suggestion is that a Setup object would have an additional constructor which takes a setup object as argument (and then performs only a subset of the initial steps) and both setups would share the data structures. This would avoid global variables but also require NEST and other systems to expose its MUSIC setups.

[mdjurfeldt]

@uahic: Please feel free to implement it this way. Given such an implementation, the step to passing the previously existing Setup object implicitly is small if this eventually would seem to be desired.

Den 7 sep. 2016 9:06 PM skrev "Martin Schulze" notifications@github.com:

@mdjurfeldt https://github.com/mdjurfeldt ok, I am looking right now into it; another suggestion is that a Setup object would have an additional constructor which takes a setup object as argument (and then performs only a subset of the initial steps) and both setups would share the data structures. This would avoid global variables but also require NEST and other systems to expose its MUSIC setups.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/INCF/MUSIC/pull/23#issuecomment-245383856, or mute the thread https://github.com/notifications/unsubscribe-auth/ADcfCUq4JaZ1OysO4-4Yncs6IIL9V4YAks5qnwsdgaJpZM4IBK4c .

[mdjurfeldt]

@uahic: However, I guess the whole point of providing the ability to work with multiple setup objects without having to pass preexisting ones explicitly is to provide a kind of modularity---it allows the user to implement a MUSIC-aware code unit/module using the MUSIC API which can be loaded into an application which already uses MUSIC. This would allow for a modular handling of the Setup phase, but, obviously, the simulation loop only can exist in one piece of code.

Sorry for arriving at this so late, but I start to agree that everything becomes cleaner if only one piece of code owns a single Setup object.

What about doing the following for now (essentially along with what you have already suggested):

1. We equip the NEST python binding with the possibility to request Setup and Runtime objects (which are "owned" by NEST but which we are allowed to use in parallel to NEST).
2. We add a method "init" to the Setup object by which we can initialize the argument list (similar to what the Setup constructor currently does).
3. We skip the multiple Setup object implementation with shared underlying data structure (for now; sorry).

?

[mdjurfeldt]

@uahic: To clarify last comment which was not fully developed: If we are forced to pass the extra argument = a preexisting Setup object, then why not simply use it directly, thus avoiding the need to at all have multiple objects sharing a data structure?

[uahic]

I didnt know if you had use-cases in mind that I havent thought of but yeah that is the straight forward solution and the reason why I asked just to add the corresponding interface functions to NEST

[mdjurfeldt]

OK---let's close this pull request then?

Den 7 sep. 2016 23:36 skrev "Martin Schulze" notifications@github.com:

I didnt know if you had use-cases in mind that I havent thought of but yeah that is the straight forward solution and the reason why I asked just to add the corresponding interface functions to NEST

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/INCF/MUSIC/pull/23#issuecomment-245427230, or mute the thread https://github.com/notifications/unsubscribe-auth/ADcfCYU0ZVq8rAwEipykd09eACWVE8x7ks5qny5TgaJpZM4IBK4c .