Skip to content

Conversation

@lisankim0321
Copy link

For better selection of upper and lower bounds of the integral calculation in get_auc, find_all_minima now has global minima in the array.

@rmatsum836
Copy link
Collaborator

Thanks! Could you add a test case that checks that the minima is correctly grabbed? This would be similar to the partial VHFs we talked about previously.

@rmatsum836
Copy link
Collaborator

Some functions you could try grabbing the minima of: x^2, 2^x, sqrt(x), logit (https://docs.scipy.org/doc/scipy/reference/generated/scipy.special.logit.html).

Copy link
Collaborator

@rmatsum836 rmatsum836 left a comment

Choose a reason for hiding this comment

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

@jacoblumpkins could you also quickly review? Thanks!

global_min = min(arr)
global_r = np.where(arr == global_min)[0]
minima = np.append(minima, global_r)
minima.sort()
Copy link
Contributor

Choose a reason for hiding this comment

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

If the global minima is also a local minima, would this not create a duplicate entry in the result array? Other than that, this look good

Copy link
Collaborator

Choose a reason for hiding this comment

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

@lisankim0321 could you double-check this for the x^2 function?

The reason for adding this is that the checks variable on line 60 doesn't correctly capture cases when a curve starts at the global minima and continues to increase as x increases.

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