HERA-Team / hera_corr_f

HERA F-Engine on SNAP
1 stars 5 forks source link

Feng noise pfb fix #90

Closed jack-h closed 2 years ago

jack-h commented 2 years ago

snap_fengine_2022-02-16_1145.fpg was built with an out-of-date, un-bugfixed PFB, which mangled data from different ADC inputs together.

This PR:

Tests are:

  1. Set all 6 ADC inputs to use generated noise with a common seed. Verify all their cross-correlations have 0 imaginary part
  2. Zero out a random selection of ADC inputs, and verify that correlations with these inputs are zero, and correlations not including these inputs (which are driven by noise) are non-zero
  3. Drive all ADC inputs with common noise and apply random delays of a small number of clock cycles to each stream. Verify that the phase of the cross correlations between streams has the expected phase slopes (within some fairly arbitrarily chosen tolerance)
AaronParsons commented 2 years ago

To be clear, the tests you list have not been run yet?

On Sat, Mar 5, 2022 at 7:22 AM Jack Hickish @.***> wrote:

@jack-h https://github.com/jack-h requested your review on: #90 https://github.com/HERA-Team/hera_corr_f/pull/90 Feng noise pfb fix.

— Reply to this email directly, view it on GitHub https://github.com/HERA-Team/hera_corr_f/pull/90#event-6189548696, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAKUG56I7XNBJMPCVS3Z3TU6N3ZXANCNFSM5P72A3BA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because your review was requested.Message ID: @.***>

-- Aaron Parsons

510-406-4322 (cell) Campbell Hall 425, UCB

david-macmahon commented 2 years ago

I'm pretty sure Jack is referring to new test scripts he added that use the internal noise source and the internal mini-correlator. I think he did run these new tests on the old design where they expose the bug and the new design where the bug is no longer seen.

jack-h commented 2 years ago

Yes, what Dave said. The tests pass on the firmware I've just added to git (see commit afcacb4dd . They fail on the firmware I was using before.

On Sat, 5 Mar 2022, 18:45 david-macmahon, @.***> wrote:

I'm pretty sure Jack is referring to new test scripts he added that use the internal noise source and the internal mini-correlator. I think he did run these new tests on the old design where they expose the bug and the new design where the bug is no longer seen.

— Reply to this email directly, view it on GitHub https://github.com/HERA-Team/hera_corr_f/pull/90#issuecomment-1059813586, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKWM7BLUGKKDIP7VUVFLJTU6OTVFANCNFSM5P72A3BA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you were mentioned. Message ID: @.***>