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

different values for n_min_threshold dependent on thresholds #18

Closed
JuliaKukulies opened this issue Jul 3, 2019 · 5 comments
Closed
Assignees
Labels
enhancement Addition of new features, or improved functionality of existing features
Milestone

Comments

@JuliaKukulies
Copy link
Member

JuliaKukulies commented Jul 3, 2019

After some discussions, @freemansw1 and I came to the conclusion that would be useful and rather easy to allow different minimum areas for different thresholds in the multithreshold feature detection. More specifically, that would mean to make n_min_threshold a dict/list that has to have the same number of elements as the provided threshold values (see suggestion @freemansw1 below).

The idea behind it is that more extreme threshold values often are inherently associated with a smaller areas (e.g. the convective core/ the region with highest rain rates/ the region with coldest cloud temperatures of a cloud system).

@freemansw1 freemansw1 added the enhancement Addition of new features, or improved functionality of existing features label Jan 19, 2022
@freemansw1
Copy link
Member

I think these are both excellent ideas. For 1, we could make n_min_threshold in feature_detection_multithreshold_timestep optionally a dict or list, with each threshold value assigned to a different number of minimum points. I think this would be a relatively easy change, although we would want to make sure to do some input validation to ensure that we have a n_min_threshold for each threshold value.

For 2, this should be a relatively easy change to add and would be of value especially for those with large datasets. I can start working on a PR to add that once we eventually get #57 (and #60 ) merged.

@JuliaKukulies
Copy link
Member Author

Great, that sounds good! Agree about your point on input validation. We could allow either a single value in which case n_min_threshold will the same for all threshold values or a list/dict in which case it has to match the number of given thresholds. I can have a look at this.

@JuliaKukulies
Copy link
Member Author

I am on this one for v1.5. PR will be ready by tomorrow or Friday!

@freemansw1
Copy link
Member

Fantastic, looking forward to it!

@JuliaKukulies
Copy link
Member Author

Has been solved with #208

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Addition of new features, or improved functionality of existing features
Projects
None yet
Development

No branches or pull requests

3 participants