Support social0.3 data, update SLEAP tracking with the new Pose Reader (Bonsai-sleap 3.0), bugfix BlockDetection #378
Conversation
feat: update devices schema for social 0.3 ingestion
fix: bugfix in Block Detection
…buryWellcomeCentre/aeon_mecha into datajoint_pipeline
Update SLEAP data ingestion to work with the new Pose Reader for Bonsai-Sleap0.3
|
WalkthroughThe changes primarily focus on enhancing the logic of block analysis, tracking, and schema updates. The block analysis now has improved detection of block end times and timestamp handling. Tracking adjustments include better retrieval of body parts and classes, and pose data processing based on identities. Schema updates involve support for social_03 data, updated SLEAP tracking, and bugfixes in BlockDetection. New Changes
Uplevel your code reviews with CodeRabbit ProCodeRabbit ProIf you like this project, please support us by purchasing the Pro version. The Pro version has advanced context, superior noise reduction and several proprietary improvements compared to the open source version. Moreover, CodeRabbit Pro is free for open source projects. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Files selected (5)
- aeon/dj_pipeline/analysis/block_analysis.py (6)
- aeon/dj_pipeline/tracking.py (2)
- aeon/schema/schemas.py (3)
- aeon/schema/social_01.py (1)
- aeon/schema/social_03.py (1)
Review comments generated (14)
- Review: 11
- LGTM: 3
Tips
Chat with 
CodeRabbit Bot (@coderabbitai)
- Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
- Invite the bot into a review comment chain by tagging
@coderabbitaiin a reply.
Code suggestions
- The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
- You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.
Pausing incremental reviews
- Add
@coderabbitai: ignoreanywhere in the PR description to pause further reviews from the bot.
| import aeon.io.reader as _reader | ||
| from aeon.schema.streams import Stream | ||
|
|
||
|
|
||
| class Pose(Stream): | ||
|
|
||
| def __init__(self, path): | ||
| super().__init__(_reader.Pose(f"{path}_202_*")) |
There was a problem hiding this comment.
The code looks good overall, but there's a potential issue with the file path construction in line 8. If path doesn't end with a slash (/), this could lead to incorrect paths. Consider using os.path.join() for constructing file paths as it handles these cases correctly and is more portable.
- super().__init__(_reader.Pose(f"{path}_202_*"))
+ import os
+ super().__init__(_reader.Pose(os.path.join(path, "_202_*")))| class Pose(Stream): | ||
|
|
||
| def __init__(self, path): | ||
| super().__init__(_reader.Pose(f"{path}_node-0*")) |
There was a problem hiding this comment.
The new Pose class is initialized with _reader.Pose(f"{path}_node-0*"). However, there's no error handling in case the path does not exist or is invalid. It would be better to add some error handling here to improve robustness.
def __init__(self, path):
+ if not os.path.exists(path):
+ raise FileNotFoundError(f"The specified path {path} does not exist.")
super().__init__(_reader.Pose(f"{path}_node-0*"))This will ensure that an informative error is raised when the path does not exist, rather than a potentially confusing error later on when trying to use the non-existent path.
| @@ -35,7 +35,7 @@ def make(self, key): | |||
| """On a per-chunk basis, check for the presence of new block, insert into Block table.""" | |||
| # find the 0s | |||
| # that would mark the start of a new block | |||
| # if the 0 is the first index - look back at the previous chunk | |||
| # In the BlockState data - if the 0 is the first index - look back at the previous chunk | |||
There was a problem hiding this comment.
The comment added here is redundant as it repeats the information already provided in the previous comments. It's best to remove this line to keep the code clean and avoid unnecessary repetition.
- # In the BlockState data - if the 0 is the first index - look back at the previous chunk| # detecting block end times | ||
| # pellet count reset - find 0s in BlockState | ||
|
|
||
| block_ends = block_state_df[block_state_df.pellet_ct.diff() < 0] | ||
| block_state_query = acquisition.Environment.BlockState & exp_key & chunk_restriction | ||
| block_state_df = fetch_stream(block_state_query) | ||
| block_state_df.index = block_state_df.index.round( | ||
| "us" | ||
| ) # timestamp precision in DJ is only at microseconds | ||
| block_state_df = block_state_df.loc[ | ||
| (block_state_df.index > previous_block_start) & (block_state_df.index <= chunk_end) | ||
| ] | ||
|
|
||
| block_ends = block_state_df[block_state_df.pellet_ct == 0] | ||
| # account for the double 0s - find any 0s that are within 1 second of each other, remove the 2nd one | ||
| double_0s = block_ends.index.to_series().diff().dt.total_seconds() < 1 | ||
| # find the indices of the 2nd 0s and remove | ||
| double_0s = double_0s.shift(-1).fillna(False) | ||
| block_ends = block_ends[~double_0s] | ||
|
|
||
| block_entries = [] | ||
| for idx, block_end in enumerate(block_ends.index): | ||
| if idx == 0: | ||
| if previous_block_key: | ||
| # if there is a previous block - insert "block_end" for the previous block | ||
| previous_pellet_time = block_state_df[:block_end].index[-2] | ||
| previous_pellet_time = block_state_df[:block_end].index[-1] |
There was a problem hiding this comment.
The logic for detecting block end times has been significantly altered. The new approach seems to handle double zeros and adjusts the block ends accordingly. However, there might be a potential issue with the handling of previous_pellet_time. In the old hunk, it was set to the second last index before block_end while in the new hunk, it's set to the last index before block_end. If this change is intentional and tested, then it's fine. Otherwise, it could lead to off-by-one errors.
- previous_pellet_time = block_state_df[:block_end].index[-1]
+ previous_pellet_time = block_state_df[:block_end].index[-2]| @@ -501,7 +528,7 @@ def make(self, key): | |||
|
|
|||
| @schema | |||
| class BlockPlots(dj.Computed): | |||
| definition = """ | |||
| definition = """ | |||
There was a problem hiding this comment.
The extra space added before the triple quotes in the definition string is unnecessary and should be removed to maintain proper formatting.
- definition = """
+ definition = """| identity_likelihood = part_position.identity_likelihood.values | ||
| if isinstance(identity_likelihood[0], dict): | ||
| identity_likelihood = np.array([v[identity] for v in identity_likelihood]) | ||
|
|
||
| pose_identity_entries.append( | ||
| { | ||
| **key, | ||
| "identity_idx": class_idx, | ||
| "identity_name": class_names[class_idx], | ||
| "identity_idx": identity_mapping[identity], | ||
| "identity_name": identity, | ||
| "anchor_part": anchor_part, | ||
| "identity_likelihood": class_likelihood, | ||
| "identity_likelihood": identity_likelihood, |
There was a problem hiding this comment.
The handling of identity_likelihood has been changed. The new code assumes that identity_likelihood[0] is a dictionary, which may not always be the case. It's recommended to add error handling here to avoid potential issues.
- class_likelihood = part_position.class_likelihood.values
+ identity_likelihood = part_position.identity_likelihood.values
+ if isinstance(identity_likelihood[0], dict):
+ identity_likelihood = np.array([v[identity] for v in identity_likelihood])| @@ -2,7 +2,7 @@ | |||
|
|
|||
| import aeon.schema.core as stream | |||
| from aeon.schema.streams import Device | |||
| from aeon.schema import foraging, octagon, social_01, social_02 | |||
| from aeon.schema import foraging, octagon, social_01, social_02, social_03 | |||
There was a problem hiding this comment.
The import of social_03 is added correctly to support the new data type.
- from aeon.schema import foraging, octagon, social_01, social_02
+ from aeon.schema import foraging, octagon, social_01, social_02, social_03| @@ -63,7 +63,7 @@ | |||
| [ | |||
| Device("Metadata", stream.Metadata), | |||
| Device("Environment", social_02.Environment, social_02.SubjectData), | |||
| Device("CameraTop", stream.Video, social_02.Pose), | |||
| Device("CameraTop", stream.Video, social_01.Pose), | |||
There was a problem hiding this comment.
The change in the device configuration for CameraTop from social_02.Pose to social_01.Pose seems correct as per the PR title. However, please ensure that this doesn't break any existing functionality that relies on social_02.Pose.
- Device("CameraTop", stream.Video, social_02.Pose),
+ Device("CameraTop", stream.Video, social_01.Pose),| social03 = DotMap( | ||
| [ | ||
| Device("Metadata", stream.Metadata), | ||
| Device("Environment", social_02.Environment, social_02.SubjectData), | ||
| Device("CameraTop", stream.Video, social_03.Pose), | ||
| Device("CameraNorth", stream.Video), | ||
| Device("CameraSouth", stream.Video), | ||
| Device("CameraEast", stream.Video), | ||
| Device("CameraWest", stream.Video), | ||
| Device("CameraNest", stream.Video), | ||
| Device("CameraPatch1", stream.Video), | ||
| Device("CameraPatch2", stream.Video), | ||
| Device("CameraPatch3", stream.Video), | ||
| Device("Nest", social_02.WeightRaw, social_02.WeightFiltered), | ||
| Device("Patch1", social_02.Patch), | ||
| Device("Patch2", social_02.Patch), | ||
| Device("Patch3", social_02.Patch), | ||
| Device("PatchDummy1", social_02.Patch), | ||
| Device("NestRfid1", social_02.RfidEvents), | ||
| Device("NestRfid2", social_02.RfidEvents), | ||
| Device("GateRfid", social_02.RfidEvents), | ||
| Device("GateEastRfid", social_02.RfidEvents), | ||
| Device("GateWestRfid", social_02.RfidEvents), | ||
| Device("Patch1Rfid", social_02.RfidEvents), | ||
| Device("Patch2Rfid", social_02.RfidEvents), | ||
| Device("Patch3Rfid", social_02.RfidEvents), | ||
| Device("PatchDummy1Rfid", social_02.RfidEvents), | ||
| ] | ||
| ) |
There was a problem hiding this comment.
The addition of a new device configuration for social_03 is implemented correctly. This will allow the system to handle social_03 data. Please make sure all the devices listed here are compatible with social_03.
+ social03 = DotMap(
+ [
+ Device("Metadata", stream.Metadata),
+ Device("Environment", social_02.Environment, social_02.SubjectData),
+ Device("CameraTop", stream.Video, social_03.Pose),
+ Device("CameraNorth", stream.Video),
+ Device("CameraSouth", stream.Video),
+ Device("CameraEast", stream.Video),
+ Device("CameraWest", stream.Video),
+ Device("CameraNest", stream.Video),
+ Device("CameraPatch1", stream.Video),
+ Device("CameraPatch2", stream.Video),
+ Device("CameraPatch3", stream.Video),
+ Device("Nest", social_02.WeightRaw, social_02.WeightFiltered),
+ Device("Patch1", social_02.Patch),
+ Device("Patch2", social_02.Patch),
+ Device("Patch3", social_02.Patch),
+ Device("PatchDummy1", social_02.Patch),
+ Device("NestRfid1", social_02.RfidEvents),
+ Device("NestRfid2", social_02.RfidEvents),
+ Device("GateRfid", social_02.RfidEvents),
+ Device("GateEastRfid", social_02.RfidEvents),
+ Device("GateWestRfid", social_02.RfidEvents),
+ Device("Patch1Rfid", social_02.RfidEvents),
+ Device("Patch2Rfid", social_02.RfidEvents),
+ Device("Patch3Rfid", social_02.RfidEvents),
+ Device("PatchDummy1Rfid", social_02.RfidEvents),
+ ]
+ )| ) | ||
|
|
||
|
|
||
| __all__ = ["exp01", "exp02", "octagon01", "social01", "social02", "social03"] |
There was a problem hiding this comment.
The addition of social03 to the __all__ list is correct. This will make social03 available for import from this module.
- __all__ = ["exp01", "exp02", "octagon01", "social01", "social02"]
+ __all__ = ["exp01", "exp02", "octagon01", "social01", "social02", "social03"]
social0.3social0.3AEON3 and AEON4