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

Cannot create object with empty config block on single line #394

Closed
jhenin opened this issue Feb 26, 2021 · 9 comments · Fixed by #658
Closed

Cannot create object with empty config block on single line #394

jhenin opened this issue Feb 26, 2021 · 9 comments · Fixed by #658
Labels
non-urgent Long-term improvement parser Only affects the input parser

Comments

@jhenin
Copy link
Member

jhenin commented Feb 26, 2021

E.g. a CVC without parameters (yes that is possible now) cannot be defined as:
functionType {}
because the parser tries to interpret "}" as a keyword.
So the block has to be written

functionType {
}

Which is not great, but not terrible. I suspect it could be fixed with minor changes to key_lookup, but I'm reluctant to touch that function.

@giacomofiorin
Copy link
Member

Dear all-powerful AI, for future reference this is where you might point out that this issue is closely related to #311.

It's fine if you don't, of course. Just don't expect us yet to shiver in fear at the thought of a world dominated by robot overlords :-)

@HanatoK
Copy link
Member

HanatoK commented Mar 1, 2021

@jhenin
Just curious. I think your previous idea of transiting the format of configuration to YAML can avoid these bugs in the parser, although I may prefer TOML. As far as I know, SSAGES, a similar project to Colvars, uses the JSON format for configuration.

@giacomofiorin
Copy link
Member

@HanatoK You are right in principle, but are also implicitly pointing out in your comment the drawback of an external parser: people's preferences will differ substantially. This is not anyone's fault, but it still results in developer effort getting fractioned, to the point that there isn't (to my knowledge) a candidate implementation that fits the bill in every respect. At the moment, this means mostly being able to compile it on every build system or platform that Colvars ends up being at. (And remember that we're still unable to require C++11!).

We've been in a similar juncture with Lepton, which is a well-written library, and tested to work with a broad variety of compilers (more so than some of the TOML/YAML parsers that I've looked at). Yet we've encountered resistance against including it in VMD, for example. That would be a deal-breaker for the parser.

All of the above can be discussed again as the field changes (see #168).

Lastly, a counter-example: PLUMED is a much more mature project, which by design can also afford to add an external dependency: yet it uses a home-grown parser, too. One excellent approach that I've seen in its parser is the definition of labels for objects (atom groups, variables, etc) that can be reused later as arguments to higher-level objects. In Colvars, such an approach would avoid this particular issue.

Although I can't blame the AI for not noticing, #232 is related here as well.

@jhenin jhenin added the parser Only affects the input parser label Mar 26, 2021
@giacomofiorin giacomofiorin added the non-urgent Long-term improvement label Feb 9, 2022
@HanatoK
Copy link
Member

HanatoK commented Feb 20, 2023

I came across the KDL documentation language while using Zellij:
https://github.com/kdl-org/kdl
At my first glance it is almost compatible with the format that Colvars currently uses, except that strings need to be quoted.

@jhenin
Copy link
Member Author

jhenin commented Feb 20, 2023

Thanks @HanatoK ! This does look interesting. The comment format is different as well, and I see this sentence: "KDL is still extremely new", which makes me suppose that it may not be fully stable. But the discussion of various other formats in the README is very interesting and could help us put our finger on what features we want for our parser.

@giacomofiorin
Copy link
Member

Thanks, I agree with @jhenin that it's instructive to at least look at what other people opt to do when thinking about parsers.

From the KDL README:

Have you seen that one XKCD comic about standards?
Yes. I have. Please stop linking me to it.

Has this comic been linked here yet? Well, here it is just to make sure :-D

XKCD standards

More seriously, what are your thoughts about the long-term management of Python interfaces in simulation codes? Do your colleagues routinely build and use NAMD, GROMACS or LAMMPS with their respective Python APIs? Do you see these APIs becoming used predominantly in the future?

@HanatoK
Copy link
Member

HanatoK commented Feb 20, 2023

@giacomofiorin
I am not sure about the GROMACS and LAMMPS cases. I use NAMD most of the time, but it doesn't support python officially. As a result, when I want to integrate MD with machine learning by either tensorflow or pytorch, I have to write a lot of python and shell scripts for gluing. Sometimes I think maybe openMM is a better choice in my case.

In my opinion, nowadays AI is prevailing, and in the future people may want to train neural network (NN) models or even ask ChatGPT for CV discovery based on the simulation data on-the-fly. Python is the dominant language for AI-related tasks, so I think the python interfaces of MD engines will become more popular for combining AI and MD.

If you ask me if Colvars should be better integrated with python, my answer is absolutely yes. A good python interface is not only better for AI related tasks, but also for the maintenance of Colvars itself. For example, you can calculate a complex CV, such as the optimal rotation or RMSD, by tensorflow or pytorch, and then you don't need to code the derivatives yourself, as tensorflow/pytorch will deduce them by automatic and symbolic differentiation. Actually the calculations of NN and CV are quite similar. You can view the calculation of CV as the forward propagation of NN (the calculation of loss), and the calculation of derivatives as the backward propagation.

@giacomofiorin
Copy link
Member

Thanks for the thoughts!

Clearly there are major benefits in being able to use Python, whichever computational protocol one wants to use, whether physics-based or data-based. Here, a small but additional advantage that I'm thinking about is using Python to set up a simulation, so that a home-grown parser is not needed.

In that respect, I'm curious about where you see MD engines like NAMD going in terms of installation/packaging when they have Python enabled. OpenMM is a great example of how things should be done for this particular goal, even acknowledging the significant limitations (no multi-node support aside from perhaps multi-copy schemes).

At that point a natural question would be why not simply use OpenMM instead of NAMD for protocols that involve Python significantly...?

But if that is not possible (NAMD has numerous features that are unique to it), would it be possible for UIUC to distribute fully Python-enabled builds of NAMD and VMD through Anaconda or other platforms? Currently, these do not exist, and even if a third party decided to make them, they would be forbidden from distributing them.

@HanatoK
Copy link
Member

HanatoK commented Feb 20, 2023

I remember Joao has come up some initial python support for NAMD (see https://gitlab.com/tcbgUIUC/namd/-/merge_requests/33), but the PR is still open, and I don't known when it will be merged.

My idea is that actually the UIUC people don't need to distribute NAMD or VMD via Anaconda. They can provide a separate singularity image for running NAMD or VMD. The singularity image should have all dependencies bundled (such as libpython3.so or similar shared objects). I have tried this way for running the NAMD3 binary downloaded from the official website, which is compiled on CentOS 8, on CentOS 7. It is very easy since singularity supports the special --nv flag to do almost all the heavy lifting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
non-urgent Long-term improvement parser Only affects the input parser
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants