search

pele-python / mcpele

Monte Carlo and parallel tempering routines built on the pele foundation
Other
20 stars 5 forks source link

Uniform rectangular sampling #75

Closed kjs73 closed 8 years ago

kjs73 commented 8 years ago

This adds an option to sample in a non-cubic (rectangular) domain in n dimensions (specified by the usual boxvec argument). With the previous version steps could only be taken in a cube.

js850 commented 8 years ago

it seems very confusing having this be an option with a flag. Two different probability distributions m_dist and m_dist05?

Why not only only code rectangular sampling? Then in cython you can pass boxvec = [1,1,1] for cubic sampling.

js850 commented 8 years ago

does it make sense to normalize boxvec so that the stepsize is really the stepsize?

kjs73 commented 8 years ago

I don't think normalizing the boxvector is right here. The target is just to sample configurations uniformly in a given rectangle (with the default a n-dim cube of edge length 2).

js850 commented 8 years ago

OK, just be careful in the future. If you think of this as a takestep routine with stepsize = 1.5 then this will only take steps of size 1.5 if the boxvec is normalized.

kjs73 commented 8 years ago

That is true. Added a warning to the documentation now.

smcantab commented 8 years ago

Maybe you can put it in the name? UnUniformRectangularSamplig? that way is crystal clear On 16 Feb 2016 2:56 pm, "Julian Schrenk" notifications@github.com wrote:

That is true. Added a warning to the documentation now.

— Reply to this email directly or view it on GitHub https://github.com/pele-python/mcpele/pull/75#issuecomment-184714779.

smcantab commented 8 years ago

the Un stands for unnormalised, maybe Unn is better, or anything on those lines On 16 Feb 2016 3:06 pm, "Stefano Martiniani" stefano.martiniani@gmail.com wrote:

Maybe you can put it in the name? UnUniformRectangularSamplig? that way is crystal clear On 16 Feb 2016 2:56 pm, "Julian Schrenk" notifications@github.com wrote:

That is true. Added a warning to the documentation now.

— Reply to this email directly or view it on GitHub https://github.com/pele-python/mcpele/pull/75#issuecomment-184714779.

js850 commented 8 years ago

ugh, that's an awful name. The simples thing is to just pass boxvec and no scaling parameter. that makes' things crystal clear.

Actually, you've already done that, so it seems good to me. Maybe change the name to UniformRectangularSampling

smcantab commented 8 years ago

ok I agree :) On 16 Feb 2016 3:24 pm, "Jacob Stevenson" notifications@github.com wrote:

ugh, that's an awful name. The simples thing is to just pass boxvec and no scaling parameter. that makes' things crystal clear.

Actually, you've already done that, so it seems good to me. Maybe change the name to UniformRectangularSampling

— Reply to this email directly or view it on GitHub https://github.com/pele-python/mcpele/pull/75#issuecomment-184727521.

kjs73 commented 8 years ago

Sounds good, the name is UniformRectangularSampling now.