Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

MovingAverage3 => MovingSum3 #2050

Merged
merged 1 commit into from
Jul 22, 2021
Merged

MovingAverage3 => MovingSum3 #2050

merged 1 commit into from
Jul 22, 2021

Conversation

jackkoenig
Copy link
Contributor

@jackkoenig jackkoenig commented Jul 22, 2021

The example in the README is a sum, not an average.

h/t @soronpo and @BracketMaster for discussions on Gitter: https://gitter.im/freechipsproject/chisel3?at=60f4adc0ec10653d5a49fe18, also discussed in dev: #2049

I thought about making it average over 4 cycles and using a right-shift to do a proper divide, but the purpose of the example is leading into the generalized FIR filter that follows so I kept it the same and just renamed it.

Marked 3.4.x so this change is reflected on the website.

Contributor Checklist

  • [NA] Did you add Scaladoc to every public function/method?
  • [NA] Did you add at least one test demonstrating the PR?
  • Did you delete any extraneous printlns/debugging code?
  • Did you specify the type of improvement?
  • [NA] Did you add appropriate documentation in docs/src?
  • Did you state the API impact?
  • Did you specify the code generation impact?
  • Did you request a desired merge strategy?
  • [NA] Did you add text to be included in the Release Notes for this change?

Type of Improvement

  • documentation

API Impact

No impact

Backend Code Generation Impact

No impact

Desired Merge Strategy

  • Squash

Release Notes

Reviewer Checklist (only modified by reviewer)

  • Did you add the appropriate labels?
  • Did you mark the proper milestone (3.2.x, 3.3.x, 3.4.x, 3.5.0) ?
  • Did you review?
  • Did you check whether all relevant Contributor checkboxes have been checked?
  • Did you mark as Please Merge?

The example in the README is a sum, not an average.
@jackkoenig jackkoenig added this to the 3.4.x milestone Jul 22, 2021
@jackkoenig
Copy link
Contributor Author

I'm not sure who originally wrote this example, maybe @grebe or @stevobailey have strong opinions about the name but seems more clear to call it a sum. Will also need a similar change to the bootcamp.

@stevobailey
Copy link
Contributor

Yeah one of us did it for the bootcamp. This one is a sum since the coefficients are all 1. So, nice catch!

@jackkoenig jackkoenig merged commit 3f007be into master Jul 22, 2021
mergify bot pushed a commit that referenced this pull request Jul 22, 2021
The example in the README is a sum, not an average.

(cherry picked from commit 3f007be)
@mergify mergify bot added the Backported This PR has been backported label Jul 22, 2021
mergify bot added a commit that referenced this pull request Jul 22, 2021
The example in the README is a sum, not an average.

(cherry picked from commit 3f007be)

Co-authored-by: Jack Koenig <koenig@sifive.com>
@jackkoenig jackkoenig deleted the moving-sum branch July 22, 2021 20:47
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Backported This PR has been backported
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants