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 problem with scale being a multidimensional array. #1132

Merged

Conversation

jurevreca12
Copy link

@jurevreca12 jurevreca12 commented Nov 19, 2024

In case scale is a multidimensional array (e.g. 1 x num_channels) scale[0] does not return a scalar. Instead
scale.item(0) should be used.

I had this problem on the model added bellow. It has channel-wise scaling factors. The problem was
that the match function returned an array, which then lead to errors (truth value of an array is ambiguous).

The change should not affect the rest of the code, nor does it change behavior.
Since the change is so trivial, I didn't bother adding a test. However, if this is deemed desired, I
can also add a test.

Type of change:

  • Bug fix (non-breaking change that fixes an issue)
  • Documentation update
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • A new research paper code implementation
  • Other (Specify)

Tests

None

Checklist

  • I have read the guidelines for contributing.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have made corresponding changes to the documentation.
  • My changes generate no new warnings.
  • I have installed and run pre-commit on the files
  • I have added tests that prove my fix is effective or that my feature works.

cnn_model.zip

…alar, if scale is a multidimensional array (e.g. qonnx returns array shape 1x18)
@jmitrevs jmitrevs added bug please test Trigger testing by creating local PR branch labels Nov 19, 2024
@jmitrevs
Copy link
Contributor

This looks good. Let's just double-check the pytests before merging.

@jmitrevs jmitrevs added this to the v1.0.0 milestone Nov 19, 2024
@jmitrevs jmitrevs merged commit c320f50 into fastmachinelearning:main Nov 19, 2024
8 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug please test Trigger testing by creating local PR branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants