Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

neff not respected when optional acor dependency missing #27

Open
chimaerase opened this issue Jul 12, 2022 · 1 comment
Open

neff not respected when optional acor dependency missing #27

chimaerase opened this issue Jul 12, 2022 · 1 comment

Comments

@chimaerase
Copy link
Contributor

Reading through the body of sample(), I realize that statements about the optional acor dependency in the README, that I also propagated into log messages in #24, are not correct. The README says:

Note that acor is not required to run the sampler, it simply calculates the effective chain length for output in the chain file.

But looking at the code in sample(), when acor is missing, the local Neff variable is set to zero. That means that when acor is missing, the subsequent comparison against Neff will always fail.

I suggest that a more transparent approach would be for sample() to check if parameter neff > 0 before starting the run, then fail immediately with a helpful error message if acor is missing, since it can never satisfy the request to terminate after the requested number of samples is collected. I also suggest updating statements about acor in the README and revisiting related log output from #24.

@chimaerase chimaerase changed the title sample() parameter neff is not respected when optional acor dependency missing sample() parameter neff not respected when optional acor dependency missing Jul 12, 2022
@chimaerase chimaerase changed the title sample() parameter neff not respected when optional acor dependency missing neff not respected when optional acor dependency missing Jul 12, 2022
@kdolum
Copy link
Collaborator

kdolum commented Jan 23, 2024

This is mostly fixed by #51, though perhaps it would be an improvement to fail immediately when neff is supplied but acor is not available, rather than only failing the first time we actually try to call acor.

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

No branches or pull requests

2 participants