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

Tutorial bug fix #253

Merged
merged 45 commits into from Jun 10, 2022
Merged

Tutorial bug fix #253

merged 45 commits into from Jun 10, 2022

Conversation

PhilippPlank
Copy link
Contributor

@PhilippPlank PhilippPlank commented Jun 9, 2022

Issue Number: #249

Objective of pull request: Tutorial07 should work again

Pull request checklist

Your PR fulfills the following requirements:

  • Issue created that explains the change and why it's needed
  • Tests are part of the PR (for bug fixes / features)
  • Docs reviewed and added / updated if needed (for bug fixes / features)
  • PR conforms to Coding Conventions
  • PR applys BSD 3-clause or LGPL2.1+ Licenses to all code files
  • Lint (flakeheaven lint src/lava tests/) and (bandit -r src/lava/.) pass locally
  • Build tests (pytest) passes locally

Pull request type

Please check your PR type:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation changes
  • Other (please describe):

What is the current behavior?

  • No output, tutorial does not work

What is the new behavior?

  • Outputs are generated as expected

Does this introduce a breaking change?

  • Yes
  • No

Supplemental information

PhilippPlank and others added 30 commits November 12, 2021 14:53
@PhilippPlank PhilippPlank added the 1-bug Something isn't working label Jun 9, 2022
@PhilippPlank PhilippPlank self-assigned this Jun 9, 2022
@weidel-p weidel-p self-requested a review June 10, 2022 08:42
Copy link
Contributor

@weidel-p weidel-p left a comment

Choose a reason for hiding this comment

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

This PR only changes two lines, setting the post_guard instead of the pre_guard and calling run_post_mgmt instead of run_pre_mgmt. That's fine for me.

@PhilippPlank PhilippPlank merged commit 59626ab into lava-nc:main Jun 10, 2022
@PhilippPlank PhilippPlank deleted the tutorial_bug_fix branch June 10, 2022 11:04
@PhilippPlank PhilippPlank linked an issue Jun 10, 2022 that may be closed by this pull request
2 tasks
@avitaleSon
Copy link

Is there a reason why the pre_guard and run_pre_mgmt dont work? Is this a bug or was it being called in the wrong way in Tutorial07?

@PhilippPlank
Copy link
Contributor Author

They work, but to do what the tutorial is supposed to do, we would also need the lrn_guard to return true. The pre_mgmt phase is coupled with learning enabled.
I did not want to introduce this behavior in this tutorial, so I switched to the post_mgmt phase, which does not have this coupling.

@avitaleSon
Copy link

Thanks a lot for clarifying!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1-bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cells output in tutorial 07 are empty
4 participants