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

Add chain_dim as from_tfp argument #847

Merged
merged 2 commits into from
Nov 10, 2019

Conversation

OriolAbril
Copy link
Member

I a basically pushing this PR to see if it would fix #456, I have not had time to do much testing, I'll try to in the following days. If somebody feels like giving it a try in the meantime feel free!

@OriolAbril
Copy link
Member Author

I have tried a couple of simple models and it seems to work. @junpenglao @jeffpollock9 it would be great if you could review it or simply try the code on some of your models. Possible changes that come to my mind, if they seem interesting to you please say so:

  • accept a tuple as input of chain_dim with the same length as var_names so that each variable can have the chain dim at different positions
  • add a draw_dim in addition for cases where the draw dimension is not the first

I'd like to point out that I have set the default chain_dim to None which assumes all data comes from single chain, so that it mimcs the current behaviour of from_tfp, however, maybe it would be better to have chain_dim=1 as default? I assume the first dimension is always the draw dimension and in general the second would be the chain.

Thanks a lot!

@OriolAbril
Copy link
Member Author

This should be ready to merge too. I think it will be a useful feature so we will get people testing it. The default is set to None so it is not a breaking change, everything will work the same unless chain_dim is specifically used.

@OriolAbril OriolAbril changed the title [WIP] Add chain_dim as from_tfp argument Add chain_dim as from_tfp argument Nov 10, 2019
@aloctavodia aloctavodia merged commit d2507d7 into arviz-devs:master Nov 10, 2019
@OriolAbril OriolAbril deleted the tfp_chain_arg branch November 10, 2019 00:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

from_tfp with multiple chains?
2 participants