search

INM-6 / multi-area-model

A large-scale spiking model of the vision-related areas of macaque cortex.
Other
70 stars 49 forks source link

Test suit issues #21

Closed golosio closed 3 years ago

golosio commented 3 years ago

Hi, I installed the model and all required packages (including NEST 2.14.0) on ubuntu 19.10. Running the test suite fails on most test scripts. The first error is: multiarea_model.py:139 TypeError: Not supported. Please store the matrix in a binary numpy file and define the path to the file as the parameter value. Apparently, the error is solved for all test scripts except for test_analysis.py if I replace the line if self.params['connection_params']['K_stable'] is None: with if self.params['connection_params']['K_stable']==False: The script test_analysis.py still fails with the error FAILED test_analysis.py::test_analysis - ZeroDivisionError: division by zero in the line: synchrony = variance / mean

AlexVanMeegen commented 3 years ago

Hey @golosio,

thanks for reporting the issue!

For the first part, I think your solution is correct - just to cross check, could you confirm that changing the default value of K_stable to None in default_params.py also works? Then I'll quickly correct that. Thanks!

The second looks a bit more involved. I think this is a downstream result of a bug that @jarsi fixed (#20). The bug essentially lead the downscaling feature to remove inhibitory connections. With this bug fixed, I think the (now present) inhibitory connections lead to a zero rate in the test_analysis test. @jarsi, does this sound reasonable to you?

jarsi commented 3 years ago

Yes, this sounds reasonable. I ran the test_analysis and also found that some layers do not spike at all. I think the analysis functions should not break if a population does not spike.

AlexVanMeegen commented 3 years ago

Thanks, @jarsi, I agree this should be fixed in the analysis function.

golosio commented 3 years ago

Thank you @AlexVanMeegen and @jarsi for the quick answer! I confirm that changing the default value of K_stable to None in default_params.py also works. Concerning the second problem, a possible quick solution could be to replace in analysis_helpers.py the line synchrony = variance / mean with: if (variance + mean != variance): synchrony = variance / mean else: synchrony = 0 (the first check is a standard trick from numerical recipes to check that mean / variance does not cause overflow, which could happen if mean is too small compared to variance) and the line: if (len(intervals) > 0): with: if (intervals.size > 0):

AlexVanMeegen commented 3 years ago

Thanks for checking! The second issue looks a bit involved:

  1. Which value is assigned for mean = 0? Technically, I think np.inf would be correct.
  2. There seems to be a bug, the CV is defined as standard deviation over mean.

@jarsi, I did not find any downstream method that actually uses the synchrony. Do you agree? Then we could simply fix the bug without affecting downstream stuff.

golosio commented 3 years ago

Technically, I think np.inf would be correct.

I agree, however I do not know how a np.inf value could affect the rest of the test script.... I think the most important thing is that the test should not fail when the rate of some population is zero.

AlexVanMeegen commented 3 years ago

@golosio, would you like to create a PR? Just asking for the credit assignment, I could of course also do this.

golosio commented 3 years ago

Hi @AlexVanMeegen, please go ahead and make the PR, and thank you for your help!