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

Resurrect num_quant_bins=2 in contrib.epidemiology #2509

Merged
merged 2 commits into from
May 28, 2020
Merged

Conversation

fritzo
Copy link
Member

@fritzo fritzo commented May 27, 2020

Addresses #2426

This resurrects the old compute_bin_probs(s, num_quant_bins=2). While this works horribly for general inference, I'd like to use that helper for another spline purpose.

I've also switched from case-checking to return-early-with-fallthrough-exception to make further modifications simpler.

Tested

  • added unit test cases

@fritzo
Copy link
Member Author

fritzo commented May 27, 2020

maybe this is a bad idea 🤔

@fritzo fritzo closed this May 27, 2020
@fritzo
Copy link
Member Author

fritzo commented May 27, 2020

@martinjankowiak would you mind if we resurrected this? I'd like to see how this feature interacts with other inference tricks, and it would ease my branch management if this were in dev.

@fritzo fritzo reopened this May 27, 2020

t = 1 - s

if num_quant_bins == 2:
Copy link
Collaborator

Choose a reason for hiding this comment

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

why not the smoother cubic version?

Copy link
Member Author

@fritzo fritzo May 28, 2020

Choose a reason for hiding this comment

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

I think my early attempt at a 2-point cubic spline was ill-conceived and not very smooth:
image

@martinjankowiak martinjankowiak merged commit 10f2912 into dev May 28, 2020
@fritzo fritzo deleted the sir-quant-2 branch May 31, 2020 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants