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

Affine hdf5 export (#221) #222

Merged
merged 15 commits into from
Sep 9, 2023
Merged

Conversation

ahenkes1
Copy link
Contributor

Issue Number: #221

Objective of pull request:
A network consisting of an cuba.Affine layer cannot be exported to hd5f due to missing shape information. This is caused by h5py interpreting the missing shapes ('None') as object type ('O'), which is not supported by the hd5f format.

Enable to export the cuba.Affine() layer to hdf5 by inferring the shapes from the respective Dense layer. Additionally, a test was added.

Pull request checklist

Your PR fulfills the following requirements:

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

Pull request type

Please check your PR type:

  • [ x] 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?

Cannot export cuba.Affine layer to hdf5.

What is the new behavior?

cuba.Affine layer can be exported to hdf5.

Does this introduce a breaking change?

  • Yes
  • [x ] No

Supplemental information

The test file failed with message:

    'test_cuba.py:19:80: E501 line too long (80 > 79 characters)'

Fixed by reformatting.
The test will fail, if a network consisting of an Affine layer cannot be
exported. This is the case, e.g., if the layer has no shape ('none'). In that
case, h5py will interpret it as an ('o') object, which is not supported by the
hdf5 format.
The shape of the Affine layer is set with respect to the output of the
respective Dense layer. Previously this was 'None', resulting in an error while
exporting to hdf5 due to being interpreted as object type by h5py. Now
everything works due to being set to Float.
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.

Tnaks @ahenkes1 The fix in cuba.py looks good. I have some concerns about the unittests though. Please address them.

tests/lava/lib/dl/slayer/block/test_cuba.py Show resolved Hide resolved
tests/lava/lib/dl/slayer/block/test_cuba.py Show resolved Hide resolved
@bamsumit bamsumit linked an issue Aug 16, 2023 that may be closed by this pull request
Currently, both tests are failing. This is due to 'l.394' of
'/lib/dl/slayer/block/base.py' not being executed. This has to be fixed (?).
@ahenkes1
Copy link
Contributor Author

@bamsumit I hope everything is fine now. Tests are passing on my machine!

@ahenkes1
Copy link
Contributor Author

ahenkes1 commented Sep 5, 2023

Hello @bamsumit , @PhilippPlank , is there still something to do here?

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.

Thanks @ahenkes1 for this contribution. I've merged it.

@bamsumit bamsumit merged commit 23e7abe into lava-nc:main Sep 9, 2023
5 checks passed
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.

cuba.Affine layer cannot be exported to hdf5
3 participants