-
Notifications
You must be signed in to change notification settings - Fork 41
Copies to NEURON the voltage, i_membrane_, and mechanism data. #382
Conversation
Framework in place.
@nrnhines : can we update test to check mechanism data returned by coreneuron against the neuron mechanism data? test is here: https://github.com/BlueBrain/CoreNeuron/blob/master/tests/jenkins/neuron_direct.py. Comparing few obvious variables would be sufficient I assume. |
The NEURON side contains a full test. Is that not sufficient? This is only used in the corenrn_embedded case. |
when coreneuron side is changed, the PR on coreneuron repo doesnt trigger NEURON tests. So updating neuron_direct.py on CoreNEURON side will make sure we are not introducing bug in the master branch of CoreNEURON. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nrnhines : overall looks good for me. Just left few remarks but already approving it.
} | ||
} | ||
|
||
for (NrnThreadMembList* tml = nt.tml; tml; tml = tml->next) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just a remark : we can start using auto
more : for (auto& tml = nt.tml;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would this really work here without changing NrnThreadMembList into std::vector or giving it some kind of explicit iterator operator?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
auto just deduce the type from assignment. If nt.tml
becomes container like std::vector then we have to use for(auto& tml : nt.tml)
.
double** mdata = nullptr; | ||
NrnThread& nt = nrn_threads[tid]; | ||
|
||
n = (*nrn2core_type_return_)(0, tid, data, mdata); // 0 means time |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
neuron doesn't have mechanism type 0
and that is the reason we are using first parameter0
to nrn2core_type_return_?
I wonder if we should change this to some different value (e.g. < 0
) in case above assumption change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
change this to some different value
Good idea Should add to the enum (on both NEURON and CoreNEURON side).
…rain/CoreNeuron#382) * Copies to NEURON the voltage, i_membrane_, and mechanism data. Framework in place. * voltage and i_membrane_ transfer implemented. * Data return for mechanisms. * Completes the data-return implementation. Needs further testing. * Data return for NrnThread._t. Inverse permute was reversed! * For direct mode, only call modl_reg once. CoreNEURON Repo SHA: BlueBrain/CoreNeuron@07bf5f1
Framework in place.
Interacts with neuronsimulator/nrn#717