search

greta-dev / greta.dynamics

a greta extension for modelling dynamical systems
https://greta-dev.github.io/greta.dynamics/
Other
6 stars 2 forks source link

Iterate dynamic matrix #12

Open goldingn opened 5 years ago

goldingn commented 5 years ago

This might be of interest @jdyen: an alternative to iterate_matrix that lets you to pass in a function to create the matrix at each iteration. The function is written in R, but the iterations are done in tensorflow, which should be much quicker than doing the matrix creation and matrix multiplication on the greta side.

Not fully tested yet. I'd appreciate feedback!

codecov[bot] commented 5 years ago

Codecov Report

Base: 93.02% // Head: 93.29% // Increases project coverage by +0.27% :tada:

Coverage data is based on head (09525b6) compared to base (34c43c7). Patch coverage: 93.91% of modified lines in pull request are covered.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #12 +/- ## ========================================== + Coverage 93.02% 93.29% +0.27% ========================================== Files 3 4 +1 Lines 258 373 +115 ========================================== + Hits 240 348 +108 - Misses 18 25 +7 ``` | [Impacted Files](https://codecov.io/gh/greta-dev/greta.dynamics/pull/12?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=greta-dev) | Coverage Δ | | |---|---|---| | [R/iterate\_dynamic\_matrix.R](https://codecov.io/gh/greta-dev/greta.dynamics/pull/12/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=greta-dev#diff-Ui9pdGVyYXRlX2R5bmFtaWNfbWF0cml4LlI=) | `93.91% <93.91%> (ø)` | | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=greta-dev). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=greta-dev)

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

goldingn commented 5 years ago

See the test for an example of how to use it.

goldingn commented 5 years ago

To do:

jdyen commented 5 years ago

Looks very useful. Will check it out soon(ish).

jdyen commented 5 years ago

Is there a way to pass greta variables as arguments to matrix_function? Ideally, I'd like to set this up with matrix elements as functions of parameters and predictors (e.g. survival = f(temperature), where f() is a function to be learned).

goldingn commented 5 years ago

Yes, via the dots argument. The example in tests does something like this.

They need to be named, can't be lexically scoped in, and there's no checking yet to make sure you named them correctly and passed everything in, so you'll get an obscure tensorflow error if you do it wrong!

jdyen commented 5 years ago

Great. I will fiddle with this and see if I can break it.

goldingn commented 5 years ago

Thanks! Shouldn't be too hard :)

jdyen commented 5 years ago

Two more queries:

jdyen commented 5 years ago

Do those two queries answer each other? I thought iter == niter. But if it's just a counter, could I pass in a vector x and then explicitly call x[iter] in the matrix_function?

goldingn commented 5 years ago

Yes exactly, iter is a counter, and it's there so you can do just that!

goldingn commented 5 years ago

Though actually now that I think about it, that won't work 😬

jdyen commented 5 years ago

i added a hack that maybe works?

idx <- calculate(iter + 1)
x_to_use <- x[idx]
goldingn commented 5 years ago

I'm pretty sure that hack will always pull out the same element of x.

We turn the R function into a TensorFlow function via some sorcery/code-torture that mocks up the inputs as fake greta arrays, then builda a TF graph for the function operations using those. Those dummy greta arrays are arbitrarily data greta arrays full of 0s, so when you run calculate, it turns that into a scalar 0, and indexes using that.

Two possible solutions to this problem:

  1. Change the input arguments to the function and iterate_dynamic_matrix(), so that instead of the iteration number, the function is just passed a static greta array for these additional arguments, and iterate_dynamic_matrix() is passed a list of greta arrays, one for each (possible) iteration, and on the TF side we pull out the correct greta array at each iteration and pass it in.
  2. Leave this as it is, and change the extract/replace syntax in greta so that (for a subset of the ways in which we can subset in R) we can extract/replace with greta arrays. I want to do some work on that code anyway (see https://github.com/greta-dev/greta/issues/309) so that for loops are less horrifically slow.

1 will be quicker but a less nice interface. 2 is a better long-term solution, but will take more time.

If we implement 1 now, we will probably want to change it back when 2 is done. But that's not really a problem. I think I'm leaning towards option 1 so that I can implement some new models soon.

jdyen commented 5 years ago

I've had a go at option 1, see https://github.com/jdyen/greta.dynamics/blob/iterable_matrix/R/iterate_dynamic_matrix.R.

This passes an argument called iterables to iterate_dynamic_matrix, which is passed as a static greta array and sliced inside the body() function of tf$while_loop (in tf_iterate_dynamic_matrix).

This works for a simple case (iterables is a numeric vector) but has not been tested under more complex cases. I will put some more work into it but wanted to get your thoughts on a couple of things before I proceed: