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

Correct way to deal with dialects for output of CUBE files #2579

Open
correaa opened this issue Mar 28, 2023 · 3 comments
Open

Correct way to deal with dialects for output of CUBE files #2579

correaa opened this issue Mar 28, 2023 · 3 comments

Comments

@correaa
Copy link

correaa commented Mar 28, 2023

I use CUBE files a lot for quick inspection of simulation results.
And I use the popular VESTA and VisIt applications for visualization.

To my surprise, I realized that there are two dialects of CUBE format defined https://gaussian.com/cubegen/

(a) The most usual format has the list of atoms and immediately after the density data.

(b) There is a second format uses "negative" number of atoms to indicate that there will some kind of "orbital" index (two numbers actually) in between the list and the data.

But inspecting the code, it seems that OpenBabel supports both variants for reading.
But it doesn't implement (a) for writing.

This is unfortunate because VESTA and VisIt and others do not support format (b).

I can hack the OpenBabel code to implement writing in format (a), but I wonder if there is a better way to handle subtle changes in the output format.
In any case if one doesn't introduce a whole new format, I would like to know if there is way to pass this information to the converter of the Format class.

I notice for example that there is a Flags() method in the Format class, but it seems to be for something else.

These are the lines that need to be modified to save the CUBE file in the most usual format (a).

https://github.com/openbabel/openbabel/blob/master/src/formats/gausscubeformat.cpp#L620-L628

Another leaner alternative, that doesn't require flags, is to default to format (a) when there is only one GridData count.

In any case, I would like to know if there is interest in implementing this in the code.
I can propose a merge request after asking some framework questions (testing, etc).

@correaa correaa changed the title What is the correct way to deal with dialects for output Correct way to deal with dialects for output of CUBE files Mar 28, 2023
@correaa
Copy link
Author

correaa commented Mar 29, 2023

To be specific, this is the change I propose.
#2580

It can be interpreted as it is, or if a there is better mechanism, I can change the negAtoms flag I introduced to depend on some flag passed to the format instead of the number of grid_datas count.

@baoilleach
Copy link
Member

Plugins, like the file formats, can have options. See for example https://openbabel.org/docs/dev/FileFormats/MOPAC_Cartesian_format.html, and the associated code https://github.com/openbabel/openbabel/blob/master/src/formats/mopacformat.cpp (lines 45 and 449, for example). Regarding testing, there is a test framework; it would be great if you could check in a test file along with a test.

@correaa
Copy link
Author

correaa commented Apr 6, 2023

Thank you for the pointers.
I will take a look into these options.

To not complicate things more than needed, I propose to default to the (simpler) format (a) in the case that there is only one grid associated with the molecule.
Having one grid is very common and it is also common that other programs only format (a) anyway.

What do you think?

This is implemented in this PR:
#2580

Please let me know if for some reason this is not acceptable and what I can do to fix it.

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