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

Small fix to check for complex synapse in create_dense #130

Merged
merged 13 commits into from Nov 30, 2022

Conversation

Michaeljurado42
Copy link
Contributor

@Michaeljurado42 Michaeljurado42 commented Nov 18, 2022

Issue Number: 129

Objective of pull request: The check for a complex synapse fails in create_dense

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?

In hdf5.py there is a line like this:

        if isinstance(layer_config["weight"], NetDict):

What is the new behavior?

        if "weight/imag" in layer_config.f:

Does this introduce a breaking change?

  • Yes
  • No

Supplemental information

This bug is the cause of an experimental change I made to a lava-dl fork which I forgot to remove.

Previously in a discussion post, @bamsumit discussed the possibility of having a complex_synapse key which would be used in this if-statement. However, adding this complex synapse key means that the current unit test ground truth data with .net files would have to be re-generated since they do not have this key.

This bug-fix removes the complex_synapse key and fixes the if statement that checks for complex synapses. More info given in the issue.

@Michaeljurado42 Michaeljurado42 changed the title Fix check for complex synapse in create_dense Small fix to check for complex synapse in create_dense Nov 18, 2022
@Michaeljurado42
Copy link
Contributor Author

I re-used an a previous fork for this PR. Is that okay, or should I create a new fork just for the specific change?

@bamsumit bamsumit linked an issue Nov 30, 2022 that may be closed by this pull request
10 tasks
Copy link
Contributor

@bamsumit bamsumit 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

@bamsumit bamsumit merged commit 534ce42 into lava-nc:main Nov 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Check for complex synapse fails in create_dense
3 participants