Thank you for the PR. I confirmed locally that it works on the example above.

Update

I must have produced a false result above. copy does not work, but deepcopy does, e.g.,

copy((a=Int[],b=[]))

throws an error.

Pull Request Test Coverage Report for Build 6023022641

• 0 of 1 (0.0%) changed or added relevant line in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage decreased (-0.03%) to 80.375%

Totals
Change from base Build 6018544130:	-0.03%
Covered Lines:	2232
Relevant Lines:	2777

💛 - Coveralls

Codecov Report

Patch coverage has no change and project coverage change: -0.03% ⚠️

Comparison is base (549d9b1) 80.40% compared to head (322aa1d) 80.37%.

@@            Coverage Diff             @@
##           master     #520      +/-   ##
==========================================
- Coverage   80.40%   80.37%   -0.03%     
==========================================
  Files          24       24              
  Lines        2776     2777       +1     
==========================================
  Hits         2232     2232              
- Misses        544      545       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

This PR shouldn't be necessary after #521 , no?

EDIT: Ah, sorry, I was thinking about vectorize, my bad!

Btw, how is this enough to make predict work? This should be failing when trying to convert into a MCMCChains.Chains since, AFAIK, we don't support NamedTuple.

EDIT: After running it locally I see what's happening 👍 We're just treating it as an iterable.

I'll add a test later this week.

I'm still confused why this example is even working; vectorize, for example, shouldn't be implemented. Is SequentialSamplingModels doing anything with DynamicPPL?

EDIT: Ah, found it: https://github.com/itsdfish/SequentialSamplingModels.jl/blob/728319eed7fac9c30fd2d2372834761eba97ee4f/src/type_system.jl#L91

But in this scenario, wouldn't it be better to just add a reconstruct implementation there too? These types are really breaking with what is expected of the abstract types they are sub-typing, and so I'm very worried things will just silently produce incorrect results somewhere 😕 And if we then add a default reconstruct, I'm worried this will happen in Turing too.

Of course, the better scenario, is probably to add proper support for something like NamedTupleSupport in Distributions.jl.

NamedTupleSupport random variate types from Distribution would be ideal, but that probably won't happen soon.

True, but then shouldn't we instead make a push to loosen the requirement of using <:Distribution instead? It feels like we should either

1. follow distributions and its assumptions, or
2. allow non-distributions to be used on the RHS.

If we do something that is in the "in-between" space, then this increases the likelihood of silently producing incorrect results.

@itsdfish if you're already messing with DynamicPPL-internals such as overloading vectorize, then I'd suggest you just also overload reconstruct, i.e. you add

reconstruct(d::SSM2D, r::AbstractVector) = deepcopy(r)

to the package:)

reconstruct(d::SSM2D, r::AbstractVector) = deepcopy(r)

Thank you. That sounds reasonable. I will do that. Thanks for your help!

Awesome:)

@yebai happy to close this PR then?