-
Notifications
You must be signed in to change notification settings - Fork 21
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 bug in amp33 shape introduced in #111 #166
Fix bug in amp33 shape introduced in #111 #166
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Look Good
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please be sure this does not break RCAL Unit or Regression tests.. it possibly could.
Also, changelog?
Not sure this warrants a changelog but I'll add one. |
Well if it breaks anything in RCAL then that test is broken. |
Codecov ReportPatch and project coverage have no change.
Additional details and impacted files@@ Coverage Diff @@
## main #166 +/- ##
=======================================
Coverage 96.73% 96.73%
=======================================
Files 9 9
Lines 1101 1101
=======================================
Hits 1065 1065
Misses 36 36 ☔ View full report in Codecov by Sentry. |
Yes, and we'd need to fix that test before merging this, to ensure we don't break the nightly regression tests. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
In #111, a small bug was introduced in the shape of the
amp33
shape for theramp
model. This PR corrects this.Checklist
CHANGES.rst
under the corresponding subsection