Introduce vector_getrange and vector_getranges for VarInfo

[torfjelde]

Problem

At the moment, this just defers to getrange(getmetadata(varinfo, varname), varname), which extracts the range for varname in the corresponding metadata.

However, there are two notions of "range" going around here:

1. The range in the internal vector representation which corresponds to varname.
2. The range in the vectorized representation of varinfo / metadata corresponding to varname, e.g. as obtained by values_to(varinfo, Vector).

Currently, getrange effectively corresponds to (1) in some loose sense (because getrange(::TypedVarInfo, ::VarName) actually returns the range used to access the vector representation inside the corresponding metadata).

Specifically, consider the following

@model function demo()
    y ~ Normal()
    x = Vector(undef, 2)
    x[1] ~ Normal()
    x[2] ~ Normal()
end
varinfo = DynamicPPL.VarInfo(demo())
DynamicPPL.getrange(varinfo, @varname(x[1])) # => 1 because `getrange(varinfo.metadata.x, @varname(x[1]))` => 1.

Now, this is all fine and good, however, there are cases where we actually want access to the "index that corresponds to varname when we convert the containe to a vector". In the above case, we would instead "want" the getrange call to return 2.

Examples of functionality that depends on this: TuringLang/Turing.jl#2421, and general things such as knowing which entry to use for which varname in initial_params to sample calls.

Solution

In this PR, I've added vector_length and vector_getrange for VarInfo, which implements (2) behavior rather than (1) behavior.

One laternative would be to make the current getrange functionality have behavior (2), and instead call the existing impl getrange_internal, which is arguably more inline with how it is used (+ this should not be implemented for AbstractVarInfo itself, only for the metadata containers).

@mhauru This feelsl like one you should have a look at:)

[torfjelde]

Technically the metadata containers should themselves implement vector_getrange, however this will be a bit complicated for VarNamedVector due to its inactive entries. I don't have time to complete that aspect right now, but the current implementation should work very nicely for Metadata based VarInfos 👍

[coveralls]

Pull Request Test Coverage Report for Build 12194504389

Details

• 33 of 45 (73.33%) changed or added relevant lines in 3 files are covered.
• 2 unchanged lines in 1 file lost coverage.
• Overall coverage decreased (-0.2%) to 86.326%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/threadsafe.jl	2	4	50.0%
src/varinfo.jl	30	40	75.0%

Files with Coverage Reduction	New Missed Lines	%
src/varinfo.jl	2	85.7%

Totals
Change from base Build 12158072125:	-0.2%
Covered Lines:	3668
Relevant Lines:	4249

---

💛 - Coveralls

[mhauru]

This seems like sensible functionality to have since we support converting VarInfos to Vectors. I'm wondering about the naming. I like getrange_internal for being explicit about its use. Even if we do that, I'm not sure I would call vector_getrange just getrange, since the vector nature of a VarInfo isn't so prominent that one would immediately think of it when one sees a function called getrange. I would be happy with vector_getrange and getrange_internal. Could also consider getrange_vector or vector_range, but not sure if they are any better.

[codecov]

Codecov Report

Attention: Patch coverage is 73.33333% with 12 lines in your changes missing coverage. Please review.

Project coverage is 86.32%. Comparing base (2252a9b) to head (b5e20d5).
Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
src/varinfo.jl	75.00%	10 Missing ⚠️
src/threadsafe.jl	50.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #738      +/-   ##
==========================================
- Coverage   86.48%   86.32%   -0.16%     
==========================================
  Files          35       35              
  Lines        4209     4249      +40     
==========================================
+ Hits         3640     3668      +28     
- Misses        569      581      +12     

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

[torfjelde]

I like getrange_internal for being explicit about its use. Even if we do that, I'm not sure I would call vector_getrange just getrange, since the vector nature of a VarInfo isn't so prominent that one would immediately think of it when one sees a function called getrange.

I agree with you here 👍 I think for now I'll just leave it as vector_getrange, and further renamings / consistency work can be done in a separate PR I'm thinking.

[torfjelde]

This is now passing in all cases except the current faililing x86 one.

[mhauru]

I agree with you here 👍 I think for now I'll just leave it as vector_getrange, and further renamings / consistency work can be done in a separate PR I'm thinking.

Happy with that since it's not exported.

Thanks @torfjelde!