taus='all' doesn't generate all
zpillio opened this issue · 4 comments
When taus='all' option is used some taus are missing. It happens because 'np.floor' used here:
allantools/allantools/allantools.py
Line 1713 in 8f1ed9d
In taus='all' case 'taus' are calculated from integer window sizes, and calculating back the window sizes and using 'floor' can produce duplication which is filterred later out.
Consider changing it to 'np.round' which reproduces the original window size and solves this issue.
with a one-line change I get failures of test_ns.py e.g.:
https://github.com/aewallin/allantools/actions/runs/4868282641/jobs/8681632645?pr=143
I may investigate this later - but comments/pull-requests are very welcome..
with a one-line change I get failures of test_ns.py e.g.: https://github.com/aewallin/allantools/actions/runs/4868282641/jobs/8681632645?pr=143
I may investigate this later - but comments/pull-requests are very welcome..
I think it has similar numerical reason, because the window range is validated by comparing floating points here which doesn't take account the case when it's rounded up.
allantools/allantools/allantools.py
Line 1711 in 8f1ed9d
Changing the validation to be done on the rounded window sizes makes the test pass.
I can't push my branch, because: "Permission to aewallin/allantools.git denied to zpillio." It may an issue on my side, but I'm not using github actively, and I haven't figured out, but here is a diff:
diff --git a/allantools/allantools.py b/allantools/allantools.py
index 3c90478..7ca84f2 100644
--- a/allantools/allantools.py
+++ b/allantools/allantools.py
@@ -1706,11 +1706,12 @@ def tau_generator(data, rate, taus=None, v=False, even=False, maximum_m=-1):
# mtotdev 2
# ttotdev 2
- taus_valid1 = taus < (1 / float(rate)) * float(len(data))
- taus_valid2 = taus > 0
- taus_valid3 = taus <= (1 / float(rate)) * float(maximum_m)
+ m = np.round(taus * rate)
+ taus_valid1 = m < len(data)
+ taus_valid2 = m > 0
+ taus_valid3 = m <= maximum_m
taus_valid = taus_valid1 & taus_valid2 & taus_valid3
- m = np.floor(taus[taus_valid] * rate)
+ m = m[taus_valid]
m = m[m != 0] # m is tau in units of datapoints
m = np.unique(m) # remove duplicates and sort
Thanks for the bug report and the diff !
If you want to appear as a contributor, the usual workflow on github is to fork the "allantools" repo under your own account, then perform the modifications on your fork, then issue a "pull request" (see https://docs.github.com/fr/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/creating-a-pull-request)
Otherwise one of us will do it...