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

Add accuracy tests for precise models lacking them #2314

Merged
merged 7 commits into from
Mar 8, 2022

Conversation

stinebuu
Copy link
Contributor

With @jougs PR #2294 I remembered that I started working on issue #1493, extending accuracy testing for precise models, a long time ago. This PR fixes the first part of that issue.

I have added iaf_psc_exp_ps and iaf_psc_exp_ps_lossless to test_iaf_ps_dc_accuracy.sli and test_iaf_ps_dc_t_accuracy.sli. To make the test run, I had to update the models to use expm1 + 1 instead of exp to increase accuracy.

It also seemed to me that the setting of t_spike for iaf_psc_exp_ps_lossless was wrong, so I have updated it to fit with the other models.

When it comes to the second part of issue #1493:

These tests accuracy in the presence of DC current only, which is not a very relevant test case. Could one create tests with realistic spike input using arbitrary precision C++ or Python libraries to generate reference data?

I am not quite sure how to fix this, but I wanted to already create this PR for the first part of the issue, as it is good to also test the accuracy of iaf_psc_exp_ps and iaf_psc_exp_ps_lossless.

@heplesser heplesser changed the title Add not-tested precise models to accuracy tests Add accuracy tests for precise models lacking them Mar 7, 2022
Copy link
Contributor

@heplesser heplesser left a comment

Choose a reason for hiding this comment

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

Looks good except for two tiny comment typos, see suggestions.

S_.y2_ =
V_.P20_ * ( P_.I_e_ + S_.y0_ ) + V_.P21_ex_ * S_.y1_ex_ + V_.P21_in_ * S_.y1_in_ + S_.y2_ * V_.exp_tau_m_;
// If we use S_.y2_ * std::exp( -V_.h_ms_ / P_.tau_m_ ) instead of
// V_.expm1_tau_m_ * S_.y2_ + S_.y2_ here, the accuracy decrease,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// V_.expm1_tau_m_ * S_.y2_ + S_.y2_ here, the accuracy decrease,
// V_.expm1_tau_m_ * S_.y2_ + S_.y2_ here, the accuracy decreases,

@@ -367,8 +366,11 @@ nest::iaf_psc_exp_ps_lossless::update( const Time& origin, const long from, cons
// update membrane potential
if ( not S_.is_refractory_ )
{
// If we use S_.y2_ * std::exp( -V_.h_ms_ / P_.tau_m_ ) instead of
// V_.expm1_tau_m_ * S_.y2_ + S_.y2_ here, the accuracy decrease,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// V_.expm1_tau_m_ * S_.y2_ + S_.y2_ here, the accuracy decrease,
// V_.expm1_tau_m_ * S_.y2_ + S_.y2_ here, the accuracy decreases,

} repeat
endl

exch
{
{ exch 24 setw exch <- ( ) <- } forall endl
{ exch 20 setw exch <- ( ) <- } forall endl
Copy link
Contributor

Choose a reason for hiding this comment

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

What is this number?

Copy link
Contributor

Choose a reason for hiding this comment

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

The column width for tabular output.

Copy link
Contributor

@clinssen clinssen left a comment

Choose a reason for hiding this comment

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

Cheers!

@heplesser heplesser merged commit 1ab674d into nest:master Mar 8, 2022
@stinebuu stinebuu deleted the test_precise_models branch March 8, 2022 13:46
@terhorstd terhorstd added S: Normal Handle this with default priority T: Maintenance Work to keep up the quality of the code and documentation. I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) labels Mar 18, 2022
@terhorstd terhorstd added this to To do in Models via automation Mar 18, 2022
@clinssen clinssen moved this from To do to Done in Models Nov 29, 2022
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: Maintenance Work to keep up the quality of the code and documentation.
Projects
Models
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants