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

Move to all Turing models in EpiAware to be non-vectorised #307

Closed
SamuelBrand1 opened this issue Jun 27, 2024 · 3 comments · Fixed by #369
Closed

Move to all Turing models in EpiAware to be non-vectorised #307

SamuelBrand1 opened this issue Jun 27, 2024 · 3 comments · Fixed by #369
Labels

Comments

@SamuelBrand1
Copy link
Collaborator

The problem with a generic forecast function for EpiAware #243 is that in general the Turing.forecast function doesn't interface smoothly with vectorised random variable sampling (see this discussion TuringLang/Turing.jl#2239).

In various f2f discussions (including with @dylanhmorris ) its been noted that these issues could (probably; we'd need to do some dev work) be resolved by going to a non-vectorised approach (e.g. filldist with Normal r.v.s rather than MvNormal). The downside is the potential performance hit (e.g. https://turinglang.org/docs/tutorials/docs-13-using-turing-performance-tips/ ).

If we want to do this on the way to resolving #243. I think a good step would be to first resolve #300 so we have an accurate before and after picture of performance.

@seabbs
Copy link
Contributor

seabbs commented Jun 28, 2024

its been noted that these issues could (probably; we'd need to do some dev work) be resolved by going to a non-vectorised approach

I agree this seems like it should be better supported with current functionality.

I also agree this is blocked by benchmarking which I will work on asap so we can unstick this

@SamuelBrand1
Copy link
Collaborator Author

Is this unblocked now?

@seabbs
Copy link
Contributor

seabbs commented Jul 8, 2024

I think we want the full benchmarking and integration profiling I am doing tomorrow in place first

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants