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

Give better error for non-finite values from nodes #1286

Merged
merged 1 commit into from
Apr 4, 2017

Conversation

jgosmann
Copy link
Collaborator

@jgosmann jgosmann commented Mar 29, 2017

Motivation and context:
This produces a slightly better error message when a node or direct mode ensemble produces non-finite values. By raising a SimulationError we make it clear that this is a problem that has to be solved on the user side and not some bug in the simulator (before one would get a FloatingPointError). The error message contains the name of the function (which might be <lambda> which is not that helpful, but definitely better than before if it is not a lambda).

I think for nodes it is correct to produce an error. In case of the direct mode ensemble with the function it would be nice if it would still run because it would run with non-direct mode ensembles. But to me it seems there is no logical value to set the result to and by not throwing an error it might seem like it works as intended, but does not. Like the Zen of Python says: “In the face of ambiguity, refuse the temptation to guess.”

I think we talked before about potentially implementing some sort of approximation of the neuron normalization in direct mode. This might solve this problem fully, but is also a much bigger jobs with some open questions.

Fixes #1280. Closes #1178.

Interactions with other PRs:
none

How has this been tested?
added tests

How long should this take to review?

  • Quick (less than 40 lines changed or changes are straightforward)

Types of changes:

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • I have read the CONTRIBUTING.rst document.
  • [n/a] I have updated the documentation accordingly.
  • I have included a changelog entry.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Copy link
Member

@tbekolay tbekolay left a comment

Choose a reason for hiding this comment

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

I was a bit worried about the cost of checking that all values are finite on every step of the simulation, but even for very large nodes (size_in=100000) the difference in speed is relatively small, so this LGTM. Merging!

@tbekolay
Copy link
Member

tbekolay commented Apr 4, 2017

Added a changelog entry; will merge once CI passes as long as no one has an issue with the changelog entry.

@tbekolay tbekolay merged commit 4666d55 into master Apr 4, 2017
@tbekolay tbekolay deleted the non-finite-values branch April 4, 2017 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Useful error messages for representing infinity Certain models may fail in direct mode
3 participants