Remove dependence on acor

[vhaasteren]

Acor has been giving people problems. If not installed (it's optional), things are fine, but with acor installed some users have reported errors.

Perhaps we can replace it or remove it for the next release.

[kdolum]

I think this is a good idea. The only use of acor is to stop the run after a certain number of effective samples, and in my experiences this feature is rarely used. I suggest the following:

Change the default value of Neff in initialize() to None. I don't think this feature should be used unless you explicitly ask for it. Don't do anything about effective number of samples unless the user supplies Neff. If they do, signal an error if acor is not available. Otherwise use it as we do now.

I would be inclined to remove the warning message when acor is not available, since they'll get an error if they try to use it, and if they don't it doesn't matter.

[AaronDJohnson]

There used to be an entire copy of the (thinned?) chain that was stored for use in acor. If that is still in the code, I think the whole thing should be removed. acor is not useful enough to cost so much memory when we have large numbers of parameters, in my opinion.

[AaronDJohnson]

There is an alternative that I think is worth mentioning: David Wright (a NANOGrav member) has modified acor so that it works without issue. That is a possible avenue.

[kdolum]

Yes there is a copy of the entire thinned chain. It is used for writing out the results, but I don't know if it is used for anything else. If not, we could completely remove the Neff feature and save only the last block, or save the entire chain only if Neff is set. But I'm reluctant to make major modifications at this point, because I would like to get it released quickly. I think there's also a question of whether this whole package has a long future or whether it will soon be replaced by something else. In the latter case, we don't want to spend a lot of time improving it.

[vhaasteren]

IMO our main priority here is to provide support so that PTMCMCSampler keeps working for our projects. Replacing acor with a version that works has my preference. I see that @davecwright3 created a package called encor on Pypi, so this might be an easy change. Given that they are imported identically, we will need to do proper version checks. None of that is hard to do, though. Good idea @AaronDJohnson

Major modifications could just go in a PR, like always, but like @kdolum says it probably isn't a priority for any of us here.

[davecwright3]

There's a package on conda-forge as well. I opened a PR on acor first, but there wasn't a response in time for the 15yr analyses so I forked the project. Both ceffyl and PTArcade have been using this forked version. The only changes made going from acor -> encor were in the packaging/build-system see diff

[kdolum]

@davecwright3, it's great that you made a new package with a different name from acor, because there are incompatible acor versions and causes a problem. But also there is a bug which leads to the error message RuntimeError: The autocorrelation time is too long relative to the variance in dimension 1. I take it this isn't fixed, because you said you only changed packaging and building things.
In any event, I think it's a serious misfeature in PTMCMCSampler that it decides whether to use acor, or any such package, based only on whether it is loaded and not whether you actually are concerned with getting an effective number of samples. It takes extra time and it exposes you to possible bugs. So I suggest that we switch to using encor and also make the changes I suggested above of only using it if you request it.

[kdolum]

Fixed by #51 as I outlined above. Whether you use the original acor or the new encor is not a property of PTMCMCSampler, but rather what package you have loaded, because encor installs as "acor". Thus when PTMCMCSampler says "import acor", it gets whichever you have. Neither is a dependency of PTMCMCSampler , and that's as it should be, since the most common use case doesn't use neff and so doesn't need any acor.