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

Aedat #23

Merged
merged 5 commits into from
Feb 11, 2025
Merged

Aedat #23

merged 5 commits into from
Feb 11, 2025

Conversation

aMarcireau
Copy link
Contributor

@aMarcireau aMarcireau commented Feb 2, 2025

See commits below.

Copy link
Contributor

@Jegp Jegp left a comment

Choose a reason for hiding this comment

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

Very nice work. Also with the addition of the spectogram. I can't find any major problems, so I'd be happy to merge this!

Have you tried to import the aedat files into DV GUI? Does it still error out? If so, we should reach out to the Inivation team.

Copy link
Contributor

Choose a reason for hiding this comment

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

I like the vmap-friendly approach. Good one.

Copy link
Contributor

Choose a reason for hiding this comment

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

Very very nice addition. We really should add a list of all these great features in the documentation soon...

@@ -46,23 +46,21 @@ impl Decoder {
off_value: Vec<u8>,
skip_errors: bool,
) -> PyResult<Self> {
Python::with_gil(|python| -> PyResult<Self> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why don't you need the GIL anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not 100% sure. I think that pyo3 does some magic under the hood through Rust lifetimes. They mentioned in their changelog that the recent changes to the pyo3 interface were meant to prepare for non-GIL python versions (https://peps.python.org/pep-0703/).

@@ -55,6 +55,38 @@ enum Function {
colors: (u32, u32, u32),
ts_and_polarities: Vec<(u64, neuromorphic_types::DvsPolarity)>,
},

// activity = activity * exp(-Δt / 𝜏) + 1
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need these comments?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably not but I figured that they might prove useful to someone else. I used them during development to make sure that my implementation. was correct (it can be hard to read since we are computing intermediary values during initialization, which is good for speed but obfuscates the equations a little).

The filter silences pixels whose activity is significantly larger than the most active of their four direct neighbors.
This makes a big visual difference on noisy recordings. Now the default for wiggle plots.
Multiple changes to ensure that generated aedat files can be loaded by DV. The biggest change is the addition of file data tables (essentially a data index at the end of aedat files), which DV requires.

Whilst loading and play back works, I've experienced minor issues when trying to load multiple files in a row (restarting DV is often required), when playing short files, or when enabling the loop playback function of DV. Some of these issues could come from the size of the packets generated by faery (typically bigger than DV's), a bug in faery.aedat, or a bug in DV. Since the latter is not completely open-source, we will need help from Inivation / Synsense to investigate further (if there is interest for better support).

Fixes #13.
Added the implementation for linear scale (which was missing before) and fixed a few edge cases (in particular constant event rates and no events).
@aMarcireau aMarcireau merged commit ad11b73 into main Feb 11, 2025
90 checks passed
@aMarcireau aMarcireau deleted the aedat branch February 11, 2025 20:29
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.

2 participants