Skip to content

fancy threshold to intake max foreground#4

Open
dsundarraman wants to merge 7 commits intomainfrom
max_foreground
Open

fancy threshold to intake max foreground#4
dsundarraman wants to merge 7 commits intomainfrom
max_foreground

Conversation

@dsundarraman
Copy link
Copy Markdown
Collaborator

Modified fancy threshold function to incorporate max foreground to clip hist beyond these values. Default value 0.0 not ideal but should work for the application.

@dsundarraman dsundarraman requested a review from JoOkuma March 13, 2025 21:47
Copy link
Copy Markdown
Member

@JoOkuma JoOkuma left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @dsundarraman, I left a few minor comments

dsundarraman and others added 5 commits March 14, 2025 10:17
Copy link
Copy Markdown
Member

@JoOkuma JoOkuma left a comment

Choose a reason for hiding this comment

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

Hi @ilan-theodoro, thank you for the fix.

I would suggest using the dispatch mechanism from numpy and avoiding importing the libraries as xp unless it's really necessary.
https://github.com/royerlab/dexpv2/blob/main/dexpv2/crosscorr.py is a good example of how most functions are implemented in dexpv2.

I understand that this is a cupy only fix, so it might be worth adding a function is_cupy_array to dexpv2.cuda, and even replacing this section of the code

dexpv2/dexpv2/cuda.py

Lines 137 to 138 in d13e1e0

elif cp is not None and isinstance(arr, cp.ndarray):
arr = arr.get()

This would simplify the typing.

It might be useful the fact that, np.float16 == cp.float16

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