| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
Codecov ReportAll modified and coverable lines are covered by tests ✅ Additional details and impacted files @@ Coverage Diff @@
## main #1012 +/- ##
=======================================
Coverage 97.41% 97.42%
=======================================
Files 89 89
Lines 14922 14959 +37
=======================================
+ Hits 14537 14574 +37
Misses 385 385 ☔ View full report in Codecov by Sentry. |
Sorry, something went wrong.
There was a problem hiding this comment.
@seanlaw
I have one comment. Can you please take a look and let me know what you think?
Sorry, something went wrong.
| distance_profile = core._mass( | ||
| Q, | ||
| Ts[i], | ||
| QT = core.sliding_dot_product(Q, Ts[i]) |
There was a problem hiding this comment.
Since Q and Ts[i] are UNPROCESSED, QT[idx] can get meaningless value for a non-finite subsequence that starts at idx. Better to just replace them with np.nan. So, for example, we can do something like this after computing QT (QT = core.sliding_dot_product(Q, Ts[i])):
# post-processing QT
if np.any(~np.isfinite(Q)):
QT[:] = np.nan
QT[~np.isfinite(M_Ts[i])] = np.nan
Note that QT[idx] will be ignored though eventually once it comes to computing its corresponding value in the distance profile via core.calculate_distance_profile (see a few lines below). In other words, we do not need the "post-processing of QT" but I think it makes code clearer.
Any concerns?
[Update]
Note: Since we pass inputs with non-finite values to core.calculate_distance_profile (Not only QT but also M_T), maybe I should better stop here and try to address #1011 first. Then come back and resume the work on this PR.
Sorry, something went wrong.
There was a problem hiding this comment.
I understand the need to replace the ugly Ts[Ts_idx][subseq_idx : subseq_idx + m] with Q. That's fine but
QT = core.sliding_dot_product(Q, Ts[i])
doesn't seem to require any post-processing when either Q or T contain np.nan.
Sorry, something went wrong.
There was a problem hiding this comment.
My bad! You are right. No need for post-processing QT
Sorry, something went wrong.
There was a problem hiding this comment.
maybe I should better stop here and try to address #1011 first. Then come back and resume the work on this PR.
I did an experiment and shared the result in #1011. We can either work on that and then come back here. Or, on the second thought, we can still merge this (after I resolve the merge conflicts), and then work on #1011.
What do you think?
Sorry, something went wrong.
There was a problem hiding this comment.
Since we have a merge conflict here anyways, let's fix #1011 first and then come back here and strikethrough the comment:
Note: Since we pass inputs with non-finite values to core.calculate_distance_profile (Not only QT but also M_T), maybe I should better stop here and try to address #1011 first. Then come back and resume the work on this PR.
so that we are focused on this issue/PR instead of being distracted by jumping back and forth. I am getting confused by having so many things open and so it feels disorganized
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
See #1006
This PR addresses part1, 2, and 3 as discussed here: #1006 (comment)