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

Fix SPA sequence example #259

Merged
merged 1 commit into from
Jun 22, 2020
Merged

Fix SPA sequence example #259

merged 1 commit into from
Jun 22, 2020

Conversation

arvoelke
Copy link
Contributor

@arvoelke arvoelke commented Jun 22, 2020

The stimulus wasn't connected to the state being sequenced, and so it would randomly break depending on whether the initial state was close enough to some vector in the vocabulary.

Motivation and context:
Fixes #258.

How has this been tested?
spa-sequence

Ran it locally and tried out a bunch of different seeds and longer simulation times. It always seems to do the right thing now.

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.
  • (added by Jan) I have included a changelog entry.
  • [N/A] I have added tests to cover my changes.
  • I have run the test suite locally and all tests passed.

Copy link
Collaborator

@jgosmann jgosmann left a comment

Choose a reason for hiding this comment

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

Thanks, I added a changelog entry and will merge once the build is green.

@jgosmann jgosmann added this to the 1.1.0 milestone Jun 22, 2020
@arvoelke
Copy link
Contributor Author

I don't see the changelog entry here (just the same commit as before, with a different hash, that has been force-pushed) in case that was to show up here.

The stimulus wasn't connected to the state being sequenced, and so
it would randomly break depending on whether the initial state
was close enough to some vector in the vocabulary.
@jgosmann
Copy link
Collaborator

Yeah, I forgot to add the changes to the index on my first try, but is already fixed.

@jgosmann jgosmann merged commit b2676eb into master Jun 22, 2020
@jgosmann jgosmann deleted the fix-258 branch June 22, 2020 20:26
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.

SPA sequence example is broken
2 participants