Replace VarInfo argument in MLDExt with transform status

[penelopeysm]

[anurag-mds]

Hi @penelopeysm, I'd like to work on this if it's still open!

After going through the source, here's what I'm thinking:

LogDensityFunctionWrapper in ext/DynamicPPLMarginalLogDensitiesExt.jl currently stores a full AbstractVarInfo, but as the existing comment notes, it seems to only be needed for reconstruction later.

LogDensityFunction in src/logdensityfunction.jl already has a transform_strategy field (LinkAll, UnlinkAll, or LinkSome), which captures all the transform information needed.

My proposed fix: replace the varinfo field in LogDensityFunctionWrapper with transform_strategy, and update VarInfo(mld, ...) to reconstruct a fresh VarInfo from the model and transform strategy directly.

Before I open a PR, I'd welcome any guidance on scope or approach particularly if there are edge cases or design constraints I should be aware of. Happy to adjust based on your feedback.

Thanks for your time!

[penelopeysm]

That's broadly speaking right, although you could go one step further: you don't need to store the transform strategy separately because it's already in the LDF. That means that LogDensityFunctionWrapper is just a wrapper around a LDF, which actually means that you could get rid of the wrapper entirely and just use LDF.

[anurag-mds]

Thanks @penelopeysm! That makes perfect sense since transform_strategy is already inside the LogDensityFunction, the wrapper becomes redundant.
So the plan is:

Remove LogDensityFunctionWrapper entirely
Pass ldf directly to MarginalLogDensities.MarginalLogDensity
In VarInfo(mld, ...), reconstruct using mld.logdensity.model and mld.logdensity.transform_strategy

One quick question for reconstructing the VarInfo, should I use DynamicPPL.VarInfo(model, transform_strategy) or is there a preferred way to do this?

[penelopeysm]

As for the VarInfo method, you could keep it as is for now if you wanted. But I think that the sensible option is to get rid of it and instead add a new method that constructs an initialisation strategy, probably an InitFromVector. The point of having a VarInfo is to have access to parameter values, but as noted in the docstring, all the other fields of the VarInfo are not updated, so that leads to an invalid / inconsistent state. The fix for this is to, instead of creating a new VarInfo and setting the values in it, make an initialisation strategy that doesn't actually run the model. Then if the user wants to get values, they have to use the initialisation strategy, run the model, and get it themselves. See https://turinglang.org/DynamicPPL.jl/stable/ldf/models/

[anurag-mds]

Thanks @penelopeysm! Understood remove LogDensityFunctionWrapper entirely, pass ldf directly, and replace VarInfo(mld, ...) with a method returning an InitFromVector strategy instead.
Going through the docs now to understand InitFromVector construction.
Will open a draft PR once I have a working implementation!

Thanks for the suggestion!
Will reach out to you in case of any doubts

[anurag-mds]

Should I also add something to HISTORY.md file?
As this issue is listed in v0.41 milestone

[penelopeysm]

Yes, please do! In general all PRs should have a changelog entry, unless it's absolutely trivial.

[anurag-mds]

Yes, please do! In general all PRs should have a changelog entry, unless it's absolutely trivial.

Okay, then I will do that change and after finalising the tests are working, I will create a PR shortly.