Skip to content

Fix offset when using synthetic noise with BioCAM's sparse compression - #1746

Merged
alejoe91 merged 2 commits into
NeuralEnsemble:masterfrom
b-grimaud:offset
Jul 28, 2025
Merged

Fix offset when using synthetic noise with BioCAM's sparse compression#1746
alejoe91 merged 2 commits into
NeuralEnsemble:masterfrom
b-grimaud:offset

Conversation

@b-grimaud

Copy link
Copy Markdown
Contributor

Simple fix for #1743

@zm711 zm711 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @b-grimaud. I think this makes sense since we need the offset. We are planning a release in the next couple days, so we will need to discuss this before merge. It may or may not make it in this release, but either way I think it would be more helpful if you put a comment in explaining why we need the offset of 2048 and you can even put in your github issue. something like

# offset of 2048 necessary b/c ....
corrected_data = data + 2048
return corrected_data

@zm711 zm711 added this to the 0.15.0 milestone Jul 7, 2025
@zm711

zm711 commented Jul 8, 2025

Copy link
Copy Markdown
Contributor

We will have @alejoe91 doublecheck this math since he worked with you on the last set of PRs. So you can wait to add the comment until he says its the right idea or just add the comment now before he reviews.

@b-grimaud

Copy link
Copy Markdown
Contributor Author

I added some context, hope it's clear enough !

@zm711

zm711 commented Jul 8, 2025

Copy link
Copy Markdown
Contributor

I added some context, hope it's clear enough !

That looks perfect to me. Like I said Alessio will have final say after review, but the logic makes sense to me.

@alejoe91
alejoe91 merged commit 9b370d0 into NeuralEnsemble:master Jul 28, 2025
4 checks passed
@zm711 zm711 modified the milestones: 0.15.0, 0.14.3 Jul 28, 2025
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.

3 participants