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

Coupling ramp nuinit #163

Merged
merged 3 commits into from
Apr 4, 2018

Conversation

stuart-knock
Copy link
Member

#101
This pull request addresses the erroneous differences between Coupling Map and Ramp when Ramp was set such that it should produce identical results to Map.

All examples provided in the comments to issue #101 now give consistent results.

The key functional change was a single character, from configf.next("nus"); to configf.next("nu");, in coupling_ramp.cpp.

NOTE: the difficulty identifying the source of the bug was unexpected behaviour of Configf::find(), for which a new issue has been added: #160.

  This is a work-around for a long-standing bug in CouplingRamp.
  It works by conforming to the expectations of Coupling::nuinit.

  The proximal cause of the bug was that CouplingRamp had set a
  label, "nus", that was incompatible with the label, "nu", that
  was expected by Coupling::nuinit -- which CouplingRamp inherits.
  However, ultimately, the difficulty identifying the source of
  the bug was unexpected behaviour of Configf::find(), for which
  a new issues has been added: BrainDynamicsUSYD#160.
@pausz pausz self-assigned this Apr 4, 2018
@pausz pausz added this to the Paper Publication milestone Apr 4, 2018
@pausz
Copy link
Contributor

pausz commented Apr 4, 2018

The patch in this PR fixes the numerical differences observed, but it obscures other potential sources of problems related to the initialization of variables as described in #101. See the note in #160.
As a temporary fix, it works.

@pausz pausz merged commit b1538d3 into BrainDynamicsUSYD:master Apr 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants