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

Option to run as sequence-to-one and sequence-to-sequence #13

Open
jmframe opened this issue Sep 28, 2021 · 5 comments
Open

Option to run as sequence-to-one and sequence-to-sequence #13

jmframe opened this issue Sep 28, 2021 · 5 comments
Labels
enhancement New feature or request

Comments

@jmframe
Copy link
Contributor

jmframe commented Sep 28, 2021

Short description explaining the high-level reason for the new issue.

Current behavior

The model state space persists throughout the simulation time.

Expected behavior

NeuralHydrology trains the LSTM to reset the state space and run a full sequence length of simulation at each time step.

Steps to replicate behavior (include URLs)

Either

  1. Have an option to pass the full sequence to the LSTM at each time step, which means also re-setting the state space at each time step. This option would increase run time by a factor equal to the sequence length. For example, if the training sequence length is 336 timesteps, this would require 336 calls the the model for each time step of prediction.
  2. Train the NeuralHydrology CustomLSTM and eliminate the state re-setting.

Screenshots

@madMatchstick madMatchstick added the enhancement New feature or request label Aug 5, 2022
@madMatchstick
Copy link
Contributor

@jmframe,

Are you able to provide more details on this one? Specially, are you suggesting to modify the forward pass via neuralhydrology's customlstm as defined here?

Thank you in advance, cheers!

@jmframe
Copy link
Contributor Author

jmframe commented Aug 17, 2022

I think the two options are 1) change the way the forward pass is made, yes, but not through NH, do that through the BMI, and 2) change the way the model is trained with NH to match the way we have implemented it. I think option 1 is much more reasonable, for the short term. In the long term, the thing to do is develop a BMI directly in the NH code, so there is no potential for conflict between training and forward ngen predictions.

@SnowHydrology
Copy link
Contributor

@jmframe I'm digging up old issues today. What are the advantages to resetting the state space each time step? It seems unlikely we'd want to multiply the current execution time by the sequence length unless some dramatic performance increases (i.e. better streamflow prediction) resulted. At some point, given time resources, we could retrain LSTM to eliminate the state resetting per your second recommendation.

@jmframe
Copy link
Contributor Author

jmframe commented Jan 26, 2024

Well. The advantage of resetting the state space and passing in the full sequence is that is how the model is trained, and that is what the weights of the model are trained to respond to. But, I did quite a bit of experimenting, and didn't find that it was much different, results-wise. The issue of computation and time constraints is an import one. It would be good to evaluate the performance of the large runs to see what this means in terms of extra costs.

There is a funded CIROH project to do all this, which was supposed to start in 2022, but you know the story...

@SnowHydrology
Copy link
Contributor

That's good to know @jmframe. I'll leave this issue open.

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

No branches or pull requests

3 participants