Fix: make importance resampling robust to importance samples with weight log(0)

[Schaechtle]

What does this do?

Fixes a bug in importance resampling. Before, importance resampling crashed if any samples had a weight = log(0). But that should be a fine case for importance resampling to handle. In this case, importance sampling should act like rejection sampling and never return those samples.

How was this tested?

I've added a test in a separate commit.

[Schaechtle]

@sritchie, sorry for the premature review request. The failing CLJS test makes sense to me and should be easy to fix. What's the Gen.clj-test-idiomatic way to test versions of dist that are compatible with both CLJ and CLJS?

(Not sure I track why the linter is failing 🤷‍♂️)

[sritchie]

@Schaechtle in this case let's test with gen.distribution.kixi, that works with both clj and cljs.

The linter's failing because Double/... won't work in cljs. I wish it would be more clear that it's firing for the 'cljs' side of the cljc file!

Instead you could do

(defn- neg-inf?
  [v]
  (= v ##-Inf))

which I believe will work in cljs as well.

[sritchie]

How about moving the lower when into the when-not block?

(when-not (neg-inf? log-weight)
          (vswap! log-total-weight #(logsumexp [log-weight %]))
          (when (dist/bernoulli (math/exp (- log-weight @log-total-weight)))
            (vreset! model-trace candidate-model-trace)))

Also this makes me thing that dist/bernoulli should really take the log-weight without running math/exp on it again...

[Schaechtle]

done!

[Schaechtle]

@sritchie, thanks for the quick turnaround and the suggestions. All implemented.

Also this makes me thing that dist/bernoulli should really take the log-weight without running math/exp on it again...

maybe we add a log-bernoulli?

[sritchie]

Nice work!