Skip to content

Tutorial bug fix#253

Merged
PhilippPlank merged 45 commits intolava-nc:mainfrom
PhilippPlank:tutorial_bug_fix
Jun 10, 2022
Merged

Tutorial bug fix#253
PhilippPlank merged 45 commits intolava-nc:mainfrom
PhilippPlank:tutorial_bug_fix

Conversation

@PhilippPlank
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown

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
Copy Markdown
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
Copy Markdown

Thanks a lot for clarifying!

monkin77 pushed a commit to monkin77/thesis-lava that referenced this pull request Jul 12, 2024
* Bugfix for the tutorial07_remote_memory_access.ipynb
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