Tests for importance sample creation

[am-kaiser]

Main issue:

Closes #97

Sub issues:

• closes importance sampling test: importance_sampling_from_distribution #98
• closes importance sampling test: importance_sampling_distribution_uniform_region #99
• closes fix number of returned importance samples #105

[swinter1]

Looks good.

One optional thing - Do we want to address this TODO while we are here? "num_samples_total; this should probably define the number of samples to return, and then continue generating them" - I think this would be pretty quick to do?

[am-kaiser]

Looks good.

One optional thing - Do we want to address this TODO while we are here? "num_samples_total; this should probably define the number of samples to return, and then continue generating them" - I think this would be pretty quick to do?

I had that in a separate issue but I agree it makes sense to include it now.

[am-kaiser]

Looks good.
One optional thing - Do we want to address this TODO while we are here? "num_samples_total; this should probably define the number of samples to return, and then continue generating them" - I think this would be pretty quick to do?

I had that in a separate issue but I agree it makes sense to include it now.

done

[am-kaiser]

Note: the docstring in importance_sampling.py will be improved in PR 123.

[am-kaiser]

looks great! I only had one comment

[am-kaiser]

You run into the danger of having an infinite loop here, i.e. if env_distribution_pdf never returns values > threshold you’ll loop forever. Suggested solution: add a max proposals/trials limit and raise a clear error if nothing is accepted.