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

Modifications to w_kinavg and potential deprecation of w_stateprobs #17

Open
astatide opened this issue Oct 13, 2016 · 2 comments
Open

Comments

@astatide
Copy link

Hi all,

Due to certain needs and the development of the post analysis reweighting toolset, there's an open question on how to proceed with some necessary code development.

For a quick recap, in the original west_tools, the unix philosophy was adhered to: have a tool do one thing, and do it really well. In this vein, w_kinavg and w_stateprobs were written: w_kinavg handles the averaging of kinetic calculations, and w_stateprobs handles the calculations associated with probabilities or other thermodynamic observables.

The post analysis toolkit is similar, but distinct: it handles everything to do with reweighting (and so it adheres to the unix philosophy in this sense), but tended to handle both kinetic and thermodynamic observables within one tool. This is largely due to the fact that both observables come out of the technique, and so it would be silly to have two versions of a script that ran the exact same calculation with minor modifications (for which dataset they actually wrote), effectively doubling the amount of calculation a user would have to do, simply to adhere to a philosophy.

However, with current work in the DEVELOPMENT branch, w_kinavg and w_postanalysis_reweight have come closer and closer to 'dataset parity'; that is, w_postanalysis_reweight is now outputting a lot of the same datasets that w_kinavg is, but also outputting things related to probabilities, as well.

I propose the following: deprecate w_stateprobs, and shift those calculations inside of w_kinavg (we're already doing some of those anyway in order to calculate the rates), or, more to the point, a toolset which calculates observables directly from the simulation.

Advantages:

  • w_kinavg and w_postanalysis_reweight could output the same datasets, meaning all plotting tools or further analysis could work regardless of which type of analysis you're using, without issue.
  • We're still sort of sticking to the 'unix' philosophy, except instead of 'this tool calculates observable X and does it really well', we're adhering more to 'this tool calculates observables using this particular method, and does it really well'.

Disadvantages:

  • We'd need to rewrite the plotting tools a bit, but so long as the datasets are named the same way they would have been before, it shouldn't be a big issue.
  • We'd likely want to rename w_kinavg, and by adding more functionality into it, the code becomes larger and a little harder to maintain.

There's an alternate solution, which is to simply break up w_postanalysis_reweight into kinetic and thermodynamic observables, but I'd advocate strongly against this solution, as the reweighting can be expensive and there's no need to force people to run something twice when we could be getting it 'for free'.

Please let me know what you think, or if you have any strong objections to such a path (or any alternate ideas).

PS: I've already put in a lot of flags to make certain calculations optional (such as the bootstrap). It wouldn't be too difficult to add a 'kinetics/probabilities' only set of flags that could speed up calculation if a user is only interested in one type of observable.

@astatide
Copy link
Author

As regards the 'problem' of naming w_kinavg, since the toolset would have the same functionality, we could just leave a script named w_kinavg.py that would call the 'new', renamed tool (w_direct or some such silly nonsense) and output a warning suggesting the user update their workflow/script/etc/whatever.

We're still in beta, so we could just change it outright, but it seems nicer to not break people's scripts/tutorials for the moment.

@astatide
Copy link
Author

astatide commented Oct 13, 2016

(I'd also need to 're-sort' w_kinavg, a bit, in order to gracefully integrate and extend the functionality of w_stateprobs, and likely 're-sort' w_postanalysis_reweight, as well, but that shouldn't be a major issue).

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

No branches or pull requests

1 participant