Skip to content

JOSS Review #18

@joaovaleriano

Description

@joaovaleriano

Dear @HadrienNU, thank you for the opportunity to review FOLIE (JOSS review at openjournals/joss-reviews#9748).

General critique:

FOLIE provides a general framework for maximum likelihood estimation of parameters for SDE models from time series data. It permits direct maximum likelihood estimation via nonlinear optimization based on scipy.optimize routines or through closed form estimators for Kramers-Moyal coefficients, based on scikit-learn.linear_model, and also indirect MLE through ELBO maximization or closely-related EM. While the paper presents FOLIE for the particular use of analyzing the dynamics of collective variables from molecular dynamics simulations, there are no examples of that in the repository, and in no point the implementation seems to be particularly focused on that.

Most of the development of FOLIE ended in the middle of 2024, but, since then, there have been no interactions from the community with the repository in terms of opening issues and pull requests. I imagine the main reason for that is the absence of a published papers solving complex problems using FOLIE. Added to that, even though there are some examples of usage in the repository, they are limited in scope, so we still lack a validation for the functionality of FOLIE in complex scenarios.

Regarding the intended purpose of analyzing collective variables from MD simulations, there are no specific differences in its implementation that makes it more adequate for such applications than the other packages mentioned in the paper. There is no section of the code particularly dedicated for the projection of MD trajectories to lower dimensional spaces where inference would happen. While the paper mentions a reduced flexibility of alternative packages, there are no examples of FOLIE handling problems that the alternatives would not be capable of. Also, the paper talks about projections to "low-dimensional" spaces, but the section "Langevin Models" seems to focus on the projection onto a 1D collective variable. Indeed, the examples in the repository are all focused on inference of 1D SDEs. I wonder how limiting this factor is, considering there may be significant loss of information when restricting the CV to a single variable.

While FOLIE provides a number of options for spatial dependence functions, I strongly believe that, if it means to be used by the community, there should be a clear general tutorial of how users could create their own drift and diffusion functions for their specific problems. This should also include the of how to do inference with data in different dimensionalities, as all examples of inference in the repository concern only 1D dynamics, but CVs may come in higher dimensions and the dynamics of each component may be importantly coupled. While I can see in the code that significant effort was put into the modularity of the package, as the authors refer to it, it requires more effort from the potential user for customization, and that could be helped with detailed examples of use. I personally found it hard to see an direct way to adapt FOLIE to do inference with >1-dimensional data.

Also related to examples of use, if the authors really intend for FOLIE to be used specifically in applications studying CVs from MD simulations, I believe the repository would benefit from an actual example showing that, as stated in the summary of the paper: "By leveraging FOLIE, researchers can effectively extrapolate low-dimensional kinetics from limited simulation data, enhancing their ability to understand and predict complex molecular dynamics."

Regarding broad use by the community, I believe the code could benefit from more detailed documentation/comments. just as an examples, the docstring for folie.Overdamped does not detail what the drift and diffusion functions should look like, i.e. what their inputs and outputs should be.

While I believe that a package like FOLIE could eventually be useful for the SDE inference community, I think it has not yet presented significant capabilities or conveniences that make it stand out. As it is, I would say that FOLIE could be published as a general purpose inference package for 1D SDEs, particularly through the use of Kramers-Moyal estimators, which seem to be the most tested option in the examples in the repository. If the authors want to preserve the motivation and goals of FOLIE, I would say it requires effort to: 1. make it easily adaptable to inference of >1D SDEs or make it clearer that the focus is to only study 1D dynamics, mentioning the associated limitations; 2. make it particularly useful to studying the dynamics of collective variables from molecular dynamics simulations to provide unique insight for the understanding of these systems.


Specific points

Statement of need

While the need for FOLIE is mainly motivated in terms of inference of molecular dynamics by projecting the system into lower dimensional collective variables, FOLIE is presented solely as a tool for inferring SDE models for the collective variables' dynamics, and could in principle be applied to any SDE inference problem, which makes it a competitor with a vast range of published tools, as they already mention. Despite the focus on collective variables for molecular dynamics FOLIE does not provide specific tools for building such collective variables. With this in mind, I believe the statement of need would have to be repurposed, framing FOLIE as a general package for Likelihood based inference of SDE models.

Adjacent to that, FOLIE is presented as tool for inference for low-dimensional systems. My impression is that the current apparatus for inference was only tested for 1D systems (or 1D projections of 2D systems). If FOLIE is intended to work with >1D data, I believe that examples of this should be showcased in the repository. If not, limitations associated with keeping to only one dimension should be stated.

Writing -- "Parallel Computation" section

The middle of the paragraph does not seem to be related to the section. For reference: "Furthermore, Langevin optimization can be integrated in a scheme for optimizing collective variables (Mouaffac et al., 2023), in which case a large number of model optimizations must be performed before collective variable optimization converges." I suppose this period is supposed to be a future plan, given I didn't find references to "Langevin optimization" in the repository.

Writing -- "Initial guess" section

It might be worth mentioning that the possibility of using Kramers-Moyal estimation for initial guesses is only available when the model used for inference has both drift and diffusion linear in parameters, which might be clear to the unfamiliar user. I say this in particular because, in principle, the LikelihoodEstimator can be used to handle scenarios where one cannot use the KramersMoyalEstimator.

Installation

Installation was simple, but the https://github.com/langevinmodel/folie/blob/main/README.md could be updated to mention the list of optional dependencies. This is at least mentioned in https://langevinmodel.github.io/folie/, but still does not include all possibilities of optional dependencies, for instance for running tests and plotting results.

Functionality

All tests ran successfully, but I did not manage to run most of the scripts or notebooks inside the tutorials and statistical_performances folders. Regarding the Double Well estimation notebooks, there are similar .py scripts that ran successfully, so maybe these notebooks can simply be deleted. I think some cleaning of the repository, at least in a presentation branch, would be beneficial.

Even though some examples ran successfully, I would not say they showcase a unique necessity for FOLIE, besides it being a convenient wrapper for using sklearn features to solve MLE problems via Kramers-Moyal coefficients estimation, as is done in most working examples.

First, let's focus on what successfully ran:

  • examples/plot_biasedOU.py: trivial example of 1D OU inference, seems to work fine.
  • examples/{plot_fem.py, example_em.py, plot_example.py}: Give different inference results based on data in examples/datasets/example_2d.trj, which is not documented enough to know where this data comes from and what it represents. Also there is no ground truth, so I cannot know which of these examples is giving better or worse results. Here, for curiosity, I also did a test using the fl.KramersMoyalEstimator with the linear model as in plot_example.py, and the fl.LikelihoodEstimator always gave similar results for the drift. But, for the diffusion, it strongly deviated using "L-BFGS-B". It only matched the Kramers-Moyal diffusion estimation for minimization methods "CG", "trust-constr", and "COBYLA".
  • examples/plot_free_energy.py: While results for drift and free energy inference look good, diffusion is overfit, but it's just a consequence of using splines for diffusion estimation when the true diffusion is constant. It would be interesting to have an example of the inference with the correct constant diffusion model, to see how it can improve the estimation of the free energy function. Also, the script prints a message "Analytical functions cannot be resized, check what your are doing", which is not helpful, it does not make it clear what fix is necessary in the code.
  • examples/toy_models/plot_biased_1D_Double_Well.py: Successful inference for the drift. For the diffusion, I understand that a 3rd order polynomial was used to fit a constant, leading to overfit.
  • examples/toy_models/plot_biased_2D_Double_Well.py: Gives an example of projection from 2 to 1 variable followed by inference. While inference seems successful, it is indeed very much a toy model, such that there is no dramatic information compression through this projection, and also the 2D analysis would be absolutely feasible.
  • examples/tutorials/DNAhairpin.ipynb: While it runs after downloading the necessary data, there are no ground truths to compare inference results, which, by the way, vary dramatically depending on the chosen number of points in the domain for the splines. But also it did not run the analysis of the folding/unfolding rate: ValueError: Values in 't_eval' are not properly sorted.. Considering these "comprehensive analysis tools" are mentioned as part of the key features of FOLIE, it would be good to have a working example of it.

Having these in mind, it seems that FOLIE lacks a crystal clear example of how it could be efficiently used to infer models for the dynamics of collective variables from MD simulations, and how the results could be used to uncover useful information that otherwise would be hard to obtain directly from such MD simulations. Generally, I feel like most examples could have more comments, or there could be a single example that is commented in high detail, preferably an example where FOLIE does not rely on its ready-made spatial dependency functions, showcasing how it can be customized by users for particular necessities. And also it could include the capability of inference from >1D data, if that's in the interested of the authors.

Second, I'd just like to mention what failed to run so that you are aware of it for future correction:

In examples/tutorials/:

  • 1D_DoubleWell_estimation.ipynb and 2D_DoubleWell_estimation.ipynb: both give the error TypeError: Overdamped.__init__() missing 1 required positional argument: 'drift'. I suppose FOLIE was modified after the creation of these notebooks and the notebooks were not updated.
  • Maximizing_likelihood.ipynb: The notebook is definitely incomplete, breaks at NameError: name 'data' is not defined.

In examples/statistical_performances/, neither of the notebooks ran, showing an error that I suppose would be simple for you to fix, if worth it.

Very minor -- typos in the paper

line 29: "exits" -> "exist", "differ" -> "differs"
line 42: reference Girardier2023 not properly formatted. Some other references (lines 44, 74, 84) do not have a space before the reference.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions