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

Fix caching state bug in cyclic forward passes #547

Merged
merged 2 commits into from Dec 19, 2022
Merged

Conversation

odow
Copy link
Owner

@odow odow commented Dec 18, 2022

I think there was a very subtle bug in the implementation of the cyclic state-drop-off logic. We should be storing the outgoing state of the second-to-last node (which will be the incoming state of the node that forms a cycle), not the outgoing state of the node that forms a cycle.

Then JADE should use historical inflows which are 52weeks + 1week long and things should work by default.

We could potentially improve things by removing the splice!, so that we build up a distribution of starting states, not the trajectory over time.

Closes #445

@odow
Copy link
Owner Author

odow commented Dec 18, 2022

I think I need to check the logic of #445 a bit more before merging.

@odow
Copy link
Owner Author

odow commented Dec 18, 2022

Okay. I think I understand the various options.

This can be resolved in a non-breaking way by adding a include_last_node::Bool = true in the constructor of DefaultForwardPass, and a terminates_on_cycle::Bool = false to the constructor of Historical.

@odow odow merged commit 7af1ae8 into master Dec 19, 2022
@odow odow deleted the od/cyclic-forward-pass branch December 19, 2022 01:38
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.

Starting states for infinite horizon
1 participant