ledatelescope / bifrost

A stream processing framework for high-throughput applications.
BSD 3-Clause "New" or "Revised" License
66 stars 29 forks source link

Update create a pipeline #213

Closed dentalfloss1 closed 1 year ago

dentalfloss1 commented 1 year ago

I updated the first part of Create-A-Pipeline in order to make the pipeline initialization a bit more readable. I also created a new section to read back the filterbank file and make a waterfall plot of the results.

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage has no change and project coverage change: -0.02% :warning:

Comparison is base (f90695c) 65.40% compared to head (c798382) 65.39%. Report is 29 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #213 +/- ## ========================================== - Coverage 65.40% 65.39% -0.02% ========================================== Files 67 67 Lines 5839 5840 +1 ========================================== Hits 3819 3819 - Misses 2020 2021 +1 ``` | [Files Changed](https://app.codecov.io/gh/ledatelescope/bifrost/pull/213?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=LEDA+Collaboration) | Coverage Δ | | |---|---|---| | [python/bifrost/pipeline.py](https://app.codecov.io/gh/ledatelescope/bifrost/pull/213?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=LEDA+Collaboration#diff-cHl0aG9uL2JpZnJvc3QvcGlwZWxpbmUucHk=) | `84.70% <0.00%> (-0.17%)` | :arrow_down: |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

jaycedowell commented 1 year ago

Overall this LGTM. My only comment is that there is this talk about figuring out the bpm of the song at the beginning that doesn't really seem happen. Do we want to try to fold that in or drop that?

dentalfloss1 commented 1 year ago

That sounds like a good idea, but I'm not sure how it could be executed. Perhaps look at the spectrograph for regular bumps in signal over time? Unfortunately I think that would require a specific example. I can look into it further though


From: jaycedowell @.> Sent: Wednesday, July 19, 2023 1:05 PM To: ledatelescope/bifrost @.> Cc: Sarah Chastain @.>; Author @.> Subject: Re: [ledatelescope/bifrost] Update create a pipeline (PR #213)

[EXTERNAL]

Overall this LGTM. My only comment is that there is this talk about figuring out the bpm of the song at the beginning that doesn't really seem happen. Do we want to try to fold that in or drop that?

— Reply to this email directly, view it on GitHubhttps://github.com/ledatelescope/bifrost/pull/213#issuecomment-1642606365, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AF35E63G5RLN7PM6ACDRZFDXRAVWJANCNFSM6AAAAAAZPD7WDA. You are receiving this because you authored the thread.Message ID: @.***>

jaycedowell commented 1 year ago

I'm not entirely sure how you would get to beats per minute. It could be something like summing over frequencies and then running another FFT to create a secondary spectrum.

league commented 1 year ago

Mentioning bpm as a possible application of this example was already there, so I just removed the part that seemed to commit to more than that. :) I spent a little time looking at algorithms that do estimate bpm, and wow… decided it was out of scope.

This is okay to merge, IMO.