Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ReadBufferSize should be automatically calculated #383

Open
aacuevas opened this issue Dec 11, 2024 · 5 comments
Open

ReadBufferSize should be automatically calculated #383

aacuevas opened this issue Dec 11, 2024 · 5 comments
Milestone

Comments

@aacuevas
Copy link
Collaborator

Since we know the bandwidth each device requires, it should be possible to calculate an optimum ReadBufferSize. The StartAcquisition operator could just check which ConfigureX operators are there, which devices are enabled and set an optimum value for the buffer.
The parameter would still exist, just blank (similar to the voltage parameters). If filled, then this value will override the automatic setting, useful for users that might want to fine-tune the experiment.

@cjsha
Copy link
Member

cjsha commented Dec 12, 2024

I'm curious: how would we take into account event-based data streams (i.e. digital inputs) which don't have a fixed data rate?

@jonnew
Copy link
Member

jonnew commented Dec 17, 2024

@cjsha thats a good point. The automatically calculated size should be pretty generous. In the case where there is an extreme amount of data coming in, they would need to revert back to setting the size manually. The same is true if they want to absolutely optimize closed-loop response, but in the other direction.

I gave this a shot and I ran into an issue I need to try to understand. Everything works well if I only configure the acqusition board. However, if I add a neuropixels headstage, this line is called only once and the Max read and write sizes are as if the neuropixels was not there. I have a feeling this is because the device table itself is not modified with a pass through device, and therefore something occurs out of order here.

image

The result, predictably, is that the read size is too small for the neuropixels data:

image

This is more a note for myself to track down what's going on tomorrow.

@jonnew
Copy link
Member

jonnew commented Dec 17, 2024

OK I found what was causing this issue, it was an easy fix. I just needed to pass the nullable values one layer up to StartAsync which can apply them with access to an up-to-date device table.

However, now that I have this, I think @cjsha's point is actually more important than I originally thought. The issues is more general than just a source with an unknow rate since, really, from the perspective of the Context there is no information on bandwidth of any data sources or sinks at all because none of them provide any indication of the frequency they will produce frames. So, even discounting something with variable rates like digital inputs, targeting a certain responsivity is currently not possible: e.g. targeting an update at least every 10 milliseconds.

I think adding a layer of information on into this implementation beyond ONI 1.0, such as adding an expected data rate to every device that could appear in the device table is not reasonable. However, @aacuevas , I think its reasonable think we should consider making this part of ONI 2.0. I think potentially there is something like this already pending in fact.

@jonnew
Copy link
Member

jonnew commented Dec 17, 2024

After talking through this with Aaron, we came to the conclusion that doing a constant multiple of Max Read and Write Size is such a hack, and so prone to not functioning in a variety of circumstances, that we will wait until the proper information is made available by the prosed modifications to device datasheets in open-ephys/ONI#65.

After this we can revisit this issue by adding functionary to all classes that derive from SingleDeviceFactory that takes information about their enable state and specified bandwidth into account to determine the overall bandwidth of the device table. @aacuevas has created lookup table for the required Block sizes for difference bandwidths that can be used for the basis of a lookup table.

@jonnew jonnew added this to the 0.4.3 milestone Dec 17, 2024
@aacuevas
Copy link
Collaborator Author

oni-bw

These are some empirical measurements I made back in the day for my thesis with the current hardware and a C program, so they should be a good approximation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants