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

SetDefaults on connections with wrong DataType crashes Nest #545

Closed
flinz opened this Issue Nov 11, 2016 · 2 comments

Comments

Projects
None yet
3 participants
@flinz
Contributor

flinz commented Nov 11, 2016

In the master branch calling SetDefaults on a connection with a wrong datatype causes Nest to terminate called without an active exception. In Nest 2.10 this correctly raised a NestError. Importantly, for neuron models this still works correctly, see below.

Missing we be missing something in our unittests?
I am unsure whether this is connected to #249, since this actually causes a crash not a silent fail.

Test Bash Script

echo "Try setting neuron C_m with float"
nest -c "/iaf_neuron << /C_m 10. >> SetDefaults"

echo "------"
echo "Try setting synapse tau_plus with float"
nest -c "/stdp_synapse << /tau_plus 10. >> SetDefaults"

echo "------"
echo "Try setting neuron C_m with int (should fail)"
nest -c "/iaf_neuron << /C_m 10 >> SetDefaults"

echo "------"
echo "Try setting synapse tau_plus with int (should fail)"
nest -c "/stdp_synapse << /tau_plus 10 >> SetDefaults"

In Nest 2.10 this behaves as expected

Try setting neuron C_m with float
------
Try setting synapse tau_plus with float
------
Try setting neuron C_m with int (should fail)

Nov 11 16:13:39 SetDefaults_l_D [Error]: TypeMismatch
    Expected datatype: doubletype
    Provided datatype: integertype
------
Try setting synapse tau_plus with int (should fail)

Nov 11 16:13:39 SetDefaults_l_D [Error]: TypeMismatch
    Expected datatype: doubletype
    Provided datatype: integertype

In the master branch the last command crashes Nest

Try setting neuron C_m with float
------
Try setting synapse tau_plus with float
------
Try setting neuron C_m with int (should fail)

Nov 11 16:23:52 SetDefaults_l_D [Error]: TypeMismatch
    Expected datatype: doubletype
    Provided datatype: integertype
------
Try setting synapse tau_plus with int (should fail)

terminate called without an active exception
test.sli: line 14: 41387 Abort trap: 6           nest -c "/stdp_synapse << /tau_plus 10 >> SetDefaults"
@heplesser

This comment has been minimized.

Show comment
Hide comment
@heplesser

heplesser Apr 4, 2017

Contributor

The reason is most likely that we have parallelized setting of defaults across threads see here and that this code does not handle exceptions safely inside the parallel region. This may be because the code only catches BadProperty, no other exceptions. The code also has only a single pointer to store the exception for all threads, which may lead to contention if two threads raise an exception simultaneously.

For safe handling of exception in a threaded context, see ConnBuilder: catch and store and re-throw in serial context.

Furthermore, I do not quite understand why most of the threaded context is marked as critical --- this just serializes everything.

@janhahne Could you take a look?

Contributor

heplesser commented Apr 4, 2017

The reason is most likely that we have parallelized setting of defaults across threads see here and that this code does not handle exceptions safely inside the parallel region. This may be because the code only catches BadProperty, no other exceptions. The code also has only a single pointer to store the exception for all threads, which may lead to contention if two threads raise an exception simultaneously.

For safe handling of exception in a threaded context, see ConnBuilder: catch and store and re-throw in serial context.

Furthermore, I do not quite understand why most of the threaded context is marked as critical --- this just serializes everything.

@janhahne Could you take a look?

@heplesser heplesser added S: High and removed S: Low labels Apr 4, 2017

janhahne added a commit to janhahne/nest-simulator that referenced this issue Apr 6, 2017

@heplesser heplesser added P: PR Created and removed P: Pending labels Apr 6, 2017

janhahne added a commit to janhahne/nest-simulator that referenced this issue Apr 7, 2017

@flinz

This comment has been minimized.

Show comment
Hide comment
@flinz

flinz Apr 7, 2017

Contributor

This is resolved by #704 .

Contributor

flinz commented Apr 7, 2017

This is resolved by #704 .

@flinz flinz closed this Apr 7, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment