Fix sampling rate issue when aggregating

[DradeAW]

Different versions of Kilosort output different sampling rate (sometimes rounding it to 2 decimal places, sometimes not)

This makes a crash when trying to aggregate both sortings together This PR fixes this

[DradeAW]

I don't know why the tests fail, it has nothing to do with my changes

[zm711]

updates in probeinterface that I think should be fixed now.

[zm711]

For me this seems fine. Recordings these days are between 20kHz and 30Khz so we are looking for a tolerance of 0.01 or 30_000.01 which is a fraction of a percent off from each other. @DradeAW you think this is just a float storage issue and why choose 0.01 vs 1e-1 or 1e-3?

[DradeAW]

Floating point errors can happen, but I'm not talking about that

In my case, I have multiple sortings for a single recording, and because of how some spike sorting algorithms work, the sampling frequency is sometimes rounded and sometimes not, creating a difference

[h-mayorquin]

Can we fix these in the output of the sorting algorithms at least here? This would be safer that the proposed solution.

For neuropixels I have seen variations of 0.01 that do add up with long recordings so I don't think that we should omit those differences by default.

[zm711]

Can we fix these in the output of the sorting algorithms at least here? This would be safer that the proposed solution.

But we don't control the algorithms for getting the sorting sampling rate. So you want our wrappers to change them?

[h-mayorquin]

Can we fix these in the output of the sorting algorithms at least here? This would be safer that the proposed solution.

But we don't control the algorithms for getting the sorting sampling rate. So you want our wrappers to change them?

Yes, that's what I am proposing. if we are so sure that they are equal that we want to relax the criteria here then why not just change them there? This seems safer than increasing the false positives here by relaxing the criteria.

[zm711]

For me I think the problem is that we allow fractional sampling rates (ie floats) in general. If sampling rates were always ints then rounding wouldn't be an issue.

Could you explain this more:

For neuropixels I have seen variations of 0.01 that do add up with long recordings so I don't think that we should omit those differences by default.

I am actually curious how sampling rate issues across long recordings get condensed into the singular sampling rate post a sorting. For example if you sampling rate varies in a long recording between 30_000.01, 30_000.02, 30_000.03, the sorting has to pick one value to use for its sampling rate so how do we dealing with that at the recording level.

[h-mayorquin]

[EDIT] Ok, we discussed in slack

Zach meant that all known acquisition systems have a nominal sampling rate that is always an integer.
I then mentioned that sometimes the rate needs to be adjusted by really small numbers. For example, a sampling rate of 30_000 vs 30_000.001 means that in an hour those two difference process will be off by three samples. You can set precise timestamps or correct the sampling rate but the point is that sometimes float sampling rates do matter for fine adjustment.

[zm711]

One minor edit. I assume most are nominally an integer. I'm sure there are always exceptions to this rule.

[h-mayorquin]

The sampling rate of some Axon files that I got today:

[alejoe91]

The sampling rate of some Axon files that I got today:

This is just to show that some systems don't have nominal sampling rate, right @h-mayorquin ?

I think we can merge this one

[zm711]

Actually it is two fold. One it shows that some do have truly float. But the actually convo (partially on slack) was Heberto is against this PR because it could make it easier to aggregate incorrectly. I was in favor of this because I think float issues with rounding is so common. Heberto pulled me a little closer to his side but I’ve moved more to neutral. So I think it would be helpful for the debate for @alejoe91 to comment why you are in favor.

[samuelgarcia]

Zach meant that all known acquisition systems have a nominal sampling rate that is always an integer. I then mentioned that sometimes the rate needs to be adjusted by really small numbers. For example, a sampling rate of 30_000 vs 30_000.001 means that in an hour those two difference process will be off by three samples. You can set precise timestamps or correct the sampling rate but the point is that sometimes float sampling rates do matter for fine adjustment.

In device this is not the sample rate which is an integer but the sampling period (and more often multiple of a shorter sampling period). So the sampling rate should in fact be very rarely a interger

[zm711]

That's a great point @samuelgarcia. My point was machines often have you select a nominal int (which more often for the various machines I use these days are integer sampling rates--> float periods), but the other way would be to in general have int sample periods which would often convert to float sampling rate.

I guess where are you getting the idea that machines are period based rather than sample rate based? You could do either right? I'm definitely willing to be convinced that my sample rate is actually a bias from what I have worked on myself.

But if this is true wouldn't it be better for us to store info as a sample period int rather than a rate. Because the comparison of the int period won't have float issues?

[h-mayorquin]

This is just to show that some systems don't have nominal sampling rate, right @h-mayorquin ?

I think we can merge this one

Yes, to show that the sampling rate is not always an integer which I think is a requirement for this PR to be sound. Unless I am misunderstanding.

[alejoe91]

This is just to show that some systems don't have nominal sampling rate, right @h-mayorquin ?
I think we can merge this one

Yes, to show that the sampling rate is not always an integer which I think is a requirement for this PR to be sound. Unless I am misunderstanding.

I don't think sampling frequency being ints is a requirement for the PR, since the math.isclose would work for floats too.

You can set precise timestamps or correct the sampling rate but the point is that sometimes float sampling rates do matter for fine adjustment.

This holds with float sampling frequencies too, no?