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

Music cont out proxy #280

Closed
wants to merge 42 commits into from
Closed

Conversation

uahic
Copy link
Contributor

@uahic uahic commented Mar 17, 2016

New neuron type to support forwarding continuous values via the MUSIC continuous output port.

Notes:

  • A description how it works can be found in the header file.
  • An example is provided in the /pynest/examples/music_cont_out_proxy_example directory
  • The code is not written to support the MusicManager class yet. If this is desired we need to discuss implementation details (how to encapsulate port buffers, etc.).
  • The max_buffered property for the port cannot be set by the user in the current state.
  • It might be desirable to clean up the code in the future

@tammoippen
Copy link
Contributor

Hello @uahic ,

TravisCI is complaining about the formatting of the code. Please follow the instructions on our developer space on formatting the code with clang-format.

@uahic
Copy link
Contributor Author

uahic commented Mar 23, 2016

@tammoippen I really did all the checks now and I used some vim plugins for clang-format but also did it via command line. Travis still complaints because of some line breaks but they are enforced by the screen width of 80 which is set in the .clang-format file. I am a bit lost ...

@tammoippen
Copy link
Contributor

Hello @uahic,
The formatting of some committed files does not confirm to our coding guidelines. Please, closely follow the instructions you find there, especially the Tooling section gives information on automated formatting with clang-format and local testing of the coding guidelines. The automated formatting is obligatory for accepting PR; please ensure to use clang-format version 3.6, as the formatting varies with different versions. NEST provides a .clang-format file in its sources.

@tammoippen
Copy link
Contributor

@weidel-p Can you please look into this PR? Thank you.

@uahic
Copy link
Contributor Author

uahic commented Apr 21, 2016

I found a bug in my model in case when the ports are not correctly mapped, a patch will follow ASAP

@uahic
Copy link
Contributor Author

uahic commented Apr 21, 2016

Update: the error seems to originate from a message input port (segfault). The segfault actually happens in the music lib if port names in the music conf and published names in the scripts does not match, regardless of the model

@uahic uahic closed this Apr 21, 2016
@uahic uahic reopened this Apr 22, 2016
synmodel.empty() == false && "synapse 'static_synapse' not available" );

const index synmodel_id = static_cast< index >( synmodel );
std::vector< long_t >::const_iterator t;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

long_tlong

@uahic
Copy link
Contributor Author

uahic commented Aug 17, 2016

@jougs Changed long_t and double_t to the standard type names

@gtrensch
Copy link
Contributor

gtrensch commented Sep 7, 2016

👍 music_cont_out_proxy_example successfully tested with music v1.1.15 ! There is no dependency on the custom music branch any more.

@gtrensch
Copy link
Contributor

gtrensch commented Sep 8, 2016

@uahic
In the course of setting up automated MUSIC tests for NEST (PR #471) a SLI version of your music_cont_out_proxy_example is required. It would be fantastic if you could provide a SLI example.

@uahic
Copy link
Contributor Author

uahic commented Sep 16, 2016

@gtrensch
I am willing to provide the SLI example/test case, however before digging deeper into SLI (which I have never worked with), I need a little hint from you: How do I add a custom neuron to the SLI models dict? Actually the music_cont_out_proxy is already in the modelsmodule ( within models folder) and I dont see another place where models need to be registered. Does the model miss any declaration/method or is SLI unhappy with the status dictionary arguments? Because SLI tells me that music_cont_out_proxy is unknown, whereas music_cont_in_proxy is not

@jougs
Copy link
Contributor

jougs commented Sep 16, 2016

@uahic: Can you please provide the exact error messages? Just from your verbal description it does not become clear what the problem is. I.e. what are the SLI commands you're running and what's the error message you receive? From the code it looks as if everything should work. And if it does from Python, this hints at a problem with your SLI code.

@uahic
Copy link
Contributor Author

uahic commented Sep 16, 2016

Using the SLI interpreter the output is as follows:

SLI ] /music_cont_out_proxy Create /bla Set
Sep 16 13:57:41 Create_l_i [Error]: UnknownModelName
/music_cont_out_proxy is not a known model
name. Please check the modeldict for a list
of available models.

I will recompile NEST, possibly the currently installed version does not include the proxy

@jougs
Copy link
Contributor

jougs commented Sep 16, 2016

I just compiled your branch and calling /music_cont_out_proxy Create works just fine. So your analysis is probably correct.

@jougs
Copy link
Contributor

jougs commented Sep 16, 2016

Please note that Create in SLI does not return the range of IDs as it does in PyNEST, but just the ID of the last created node. This might lead to unexpected results if you create multiple nodes in a single call.

@uahic
Copy link
Contributor Author

uahic commented Sep 16, 2016

@jougs Thanks for the hint! And yep, after re-compiling it works. Next week there should be some time to spare for creating the SLI test's/examples.

@uahic
Copy link
Contributor Author

uahic commented Oct 3, 2016

I've wrote half of the unit-test and hopefully will be able to deliver until the end of the week, currently I am pretty busy with project work and want to excuse that I did not have enough time to review Jochen's pull requests

@uahic uahic closed this Oct 3, 2016
@uahic uahic reopened this Oct 3, 2016
@heplesser heplesser added ZC: Kernel DO NOT USE THIS LABEL I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) ZP: PR Created DO NOT USE THIS LABEL S: Normal Handle this with default priority T: Enhancement New functionality, model or documentation labels Nov 17, 2016
@heplesser heplesser removed the request for review from gtrensch June 29, 2017 11:51
@gtrensch gtrensch mentioned this pull request Aug 3, 2017
@gtrensch
Copy link
Contributor

gtrensch commented Aug 3, 2017

This pull request has been replaced by PR #798 and can be closed.

@jougs jougs closed this Aug 3, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) S: Normal Handle this with default priority T: Enhancement New functionality, model or documentation ZC: Kernel DO NOT USE THIS LABEL ZP: PR Created DO NOT USE THIS LABEL
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants