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.

Sign up for GitHub

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

Jump to bottom

Ensure probe cache is cleared on reset #1324

Merged
merged 1 commit into from
Jun 22, 2017
Merged

Ensure probe cache is cleared on reset #1324

merged 1 commit into from
Jun 22, 2017

Conversation

hunse
Copy link
Collaborator

@hunse hunse commented Jun 9, 2017 •
edited
Loading

Motivation and context:

This fixes a bug, where probes were returning cached values after a simulator reset, if the simulator had been run for the same amount of time (see test).

How has this been tested?

I added a unit test.

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.
  • 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.

@hunse hunse requested review from tbekolay and jgosmann June 9, 2017 18:44
drasmuss
drasmuss approved these changes Jun 9, 2017
View reviewed changes
Copy link
Member

@drasmuss drasmuss 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. That must have been an interesting bug to track down ;)

@drasmuss
Copy link
Member

drasmuss commented Jun 9, 2017

(Note, the failing appveyor tests are due to the problem addressed in #1323, so should be fine when that is merged in)

@hunse hunse mentioned this pull request Jun 9, 2017
Merged
@hunse
Copy link
Collaborator Author

hunse commented Jun 9, 2017

That must have been an interesting bug to track down ;)

Unfortunately I haven't been keeping up with Nengo dev, otherwise I think it would have been easier. But yes, it was very fun 😝

tbekolay
tbekolay approved these changes Jun 22, 2017
View reviewed changes
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.

LGTM, thanks @hunse!

@tbekolay tbekolay merged commit 4b542a4 into master Jun 22, 2017
@tbekolay tbekolay deleted the clear-probe-cache branch June 22, 2017 20:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tbekolay tbekolay tbekolay approved these changes

@jgosmann jgosmann jgosmann approved these changes

@Seanny123 Seanny123 Seanny123 approved these changes

@drasmuss drasmuss drasmuss approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@hunse @drasmuss @tbekolay @jgosmann @Seanny123