Skip to content

Conversation

@JohannesBuchner
Copy link

Currently, the stretch move resamples again. Instead of finding a one-to-one mapping of red and blue, this can reuse points, decreasing proposal diversity.

This commit favors a one-to-one mapping.

Currently, the stretch move resamples again. Instead of finding a one-to-one mapping of red and blue, this can reuse points, decreasing proposal diversity.

This commit favors a one-to-one mapping.
this avoids complex append/concatenate code on multi-dimensional arrays
Copy link
Owner

@dfm dfm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Looks good but here are some high level comments:

  1. I'd prefer not to change default behavior. If we add this behavior, let's put it under an option that gets passed to the move. This project is pretty much in maintenance mode and I don't think it's a great idea to dramatically change default behavior like this.

  2. This won't necessarily play nicely with the nsplits and randomize_split parameters to the RedBlueMove. If nsplits != 2 then the else will always be hit, and if randomize_split is False then this will deterministically reuse the same helper walker every time. Perhaps a better approach would be to shuffle arange(Nc) and then append randint(Nc, size=(Ns-Nc,)) if Nc < Ns?

ndim = s.shape[1]
zz = ((self.a - 1.0) * random.rand(Ns) + 1) ** 2.0 / self.a
factors = (ndim - 1.0) * np.log(zz)
rint = random.randint(Nc, size=(Ns,))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps remove this line? It's no longer needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants