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

Ambient data #777

Open
wants to merge 3 commits into
base: iblrigv8dev
Choose a base branch
from
Open

Ambient data #777

wants to merge 3 commits into from

Conversation

bimac
Copy link
Contributor

@bimac bimac commented Mar 3, 2025

No description provided.

@bimac bimac requested a review from oliche March 3, 2025 17:15
@bimac bimac changed the base branch from iblrigv8 to iblrigv8dev March 3, 2025 17:15
@bimac bimac linked an issue Mar 3, 2025 that may be closed by this pull request
@coveralls
Copy link

coveralls commented Mar 3, 2025

Coverage Status

Changes unknown
when pulling fc404fd on ambient_data
into ** on iblrigv8dev**.

@bimac
Copy link
Contributor Author

bimac commented Mar 3, 2025

Regarding the choice of .bin: Both ZARR and HDF5 (via Pandas/PyTables) require Python >=3.11

Copy link
Member

@oliche oliche left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure writing binary files in this manner is a good idea. If we can't access hdf5/zarr then I would revert to CSV for the time being.

Here having just numbers in an unspecified shape and without column names is calling for trouble if we need to refactor or version this dataset.

I'll let you judge. If you wish to stick to unspecified binary I would at the very least move the reader to iblutils and document this in Alyx.

self.bpod_modules.module_write(self.ambient_module, 'R')
reply = self.bpod_modules.module_read(self.ambient_module, 12)
self.ambient_module.stop_module_relay()
data = np.frombuffer(bytes(reply), dtype=np.float16).copy()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the original data in float32 ? Why do we use float16 here ?
I'm not worried about precision but float16 is not straightforward to manipulate and we reserve the use to very large datasets. I would write float32 for user side convenience.

if self.hardware_settings.device_bpod.USE_AMBIENT_MODULE:
sensor_reading = self.bpod.get_ambient_sensor_reading()
self.ambient_sensor_table.iloc[self.trial_num] = sensor_reading
self.bpod.write_ambient_data(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes so here it is worthwhile noting that we get to open/close the file at each trial.
It is not an issue in our case, but for a much faster stream, we would rather open the file at the start of the session, and feed the file pointer to this method.

The idea being that the method can parse the type being fed, if it is a file pointer it writes directly, if it is a file path it opens / closes the file as usual.

ambient_data.tofile(f)

@staticmethod
def read_ambient_data(filepath: Path | str) -> pd.DataFrame:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would put this with the jsonable in iblutils !

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

Successfully merging this pull request may close these issues.

Record ambient sensor data to disk
3 participants