Fix acceptance rate reporting

[ortega2247]

After restarting a pydream calibration, pydream was calculating the acceptance rate as it has just begun instead of taking into account the previous iterations and number of accepted points. This PR saves the acceptance rates per calibration run and the number of accepted points to files and then used them for reference when the pydream calibration is restarted.

[alubbock]

A few small suggestions. Also wondering if there's any kind of unit test that would be appropriate here, like a pause/resume to test the file save/load. What do you think?

[alubbock]

LGTM

[alubbock]

Should the reporting files be deleted by default when PyDREAM completes? Maybe with a cleanup=True default argument?

[michael-irvin]

See my pull request here: #22

[ortega2247]

Should the reporting files be deleted by default when PyDREAM completes? Maybe with a cleanup=True default argument?

@alubbock @michael-irvin Might be better to have a save_acceptance_rate=False default argument? That way pydream doesn't spend time creating the files to delete them afterward. Also, if pydream is restarted but the acceptance rate files were not saved or were cleaned up, should we print a warning saying that it was restarted but no acceptance rates files were found?

[michael-irvin]

Also, if pydream is restarted but the acceptance rate files were not saved or were cleaned up, should we print a warning saying that it was restarted but no acceptance rates files were found?

Would this re-create the issue of acceptance rates mysteriously decreasing after the first restart?

[ortega2247]

ummm good point. Yes, this would recreate that issue as to get to obtain correctly the acceptance rates the history file is required

[ortega2247]

Ok, since the naccepts files are necessary to restart pydream and have the correct acceptance rates, we shouldn't delete them. If you @alubbock @michael-irvin are ok with this I am going to go ahead and merge this PR

[lh64]

Don't merge our own PR, that's bad form

…

On Wed, Jul 8, 2020 at 1:01 PM Oscar Ortega ***@***.***> wrote: Ok, since the naccepts files are necessary to restart pydream and have the correct acceptance rates, we shouldn't delete them. If you @alubbock <https://github.com/alubbock> @michael-irvin <https://github.com/michael-irvin> are ok with this I am going to go ahead and merge this PR — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#20 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAKK3Y5IHE7KRSWNZS524LTR2SX73ANCNFSM4NFFDGAA> .

[lh64]

*your

…

On Wed, Jul 8, 2020 at 2:21 PM Leonard Harris ***@***.***> wrote: Don't merge our own PR, that's bad form On Wed, Jul 8, 2020 at 1:01 PM Oscar Ortega ***@***.***> wrote: > Ok, since the naccepts files are necessary to restart pydream and have > the correct acceptance rates, we shouldn't delete them. If you @alubbock > <https://github.com/alubbock> @michael-irvin > <https://github.com/michael-irvin> are ok with this I am going to go > ahead and merge this PR > > — > You are receiving this because you are subscribed to this thread. > Reply to this email directly, view it on GitHub > <#20 (comment)>, or > unsubscribe > <https://github.com/notifications/unsubscribe-auth/AAKK3Y5IHE7KRSWNZS524LTR2SX73ANCNFSM4NFFDGAA> > . >

[ortega2247]

Yeah @lh64 , I think that there are not standardized best practices for merging pull requests. What most people agree on is to only merge after the reviewers have approved the PR (https://www.quora.com/Is-it-best-practice-to-merge-your-own-pull-request-or-have-someone-else-do-it-on-Github). Since @alubbock and @michael-irvin already approved the PR, and there are no new comments or concerns I thought it was fine to go ahead and merge the PR