Windows 10, Run... does not work

[Ineluki80]

Windows 10, Avogadro 1.99 Nightly 2024-02-08, avo-xtb 2024-02-13T14:00:00

Problem: Run... produces empty input.xyz files and therefore gives error in run.py: line 211

To Reproduce:

1. Click to produce Methane
2. Click Run... in Menu. Note that "Save results in" is empty and "Use Turbomole Geometry" is unchecked.
3. Click OK
   --> input.tmol in last/ ist OK, input.xyz is 0 bytes

Checking the checkbox "Use Turbomole Geometry" runs the calculation, but since "Save results in" was empty, an error in run.py: line 261 occurs.

Setting both "Use Turbomole Geometry" and "Save results in" works fine.

Solution:

1. Use default path "avo_xtb/last" when "Save results in" string is empty.
2. Haven't investigated the reason for the empty input.xyz, yet. Maybe oabel is called in the wrong way ?

[matterhorn103]

Try out the absolute most recent code.

The "Save results in" field should be automatically populated with the same path as in the "Run calculations in" field under Configure..., but with /last appended. I'm not sure how it would end up having no value. You can see the current saved value in config.json – it should either be <pluginsdirectory>/avo_xtb/last or <USER>/avo_xtb/last, unless you've manually set it.

Recent changes to Avogadro means the script can't currently automatically find the Open Babel binary. My recent commit adjusted things so that menu options that need Open Babel won't be displayed if the plugin can't find the binary, so if you use the latest code the Run... command should disappear.

You'll have to set the path to obabel in the Configure... menu manually. obabel should be in the same folder as avogadro2.exe.

Haven't investigated the reason for the empty input.xyz, yet. Maybe oabel is called in the wrong way ?

It's because obabel isn't being successfully called at all. All the other commands request xyz files from Avogadro and submit them to xtb directly, and Open Babel is first used to convert the results back to CJSON. So without Open Babel, the calculations successfully run, but the results aren't displayed. On the other hand the Run... command always requests Turbomole format from Avogadro (because only one format can be requested as it stands), and then converts it to xyz with Open Babel if "Use Turbomole geometry" wasn't selected, and submits that to xtb. So with Run..., if no Open Babel is found, the calculation doesn't even run because the conversion to xyz couldn't happen.

[Ineluki80]

I tried with todays Nightly of avogadro2 and todays avo-xtb, but I can't even get the plugin running, i.e. no menues are shown.
Still there is no error in the Avogadro2.log.

The files from the Avogadro2 Plugin-Manager do load but they seem to be an old version of avo-xtb with the menues still having no elipses.

So testing may take a while.

[matterhorn103]

I found the problem. It was to do with the formatting of the version number in plugin.json. Fixed now.

The files from the Avogadro2 Plugin-Manager do load but they seem to be an old version of avo-xtb with the menues still having no elipses.

Yes, the version available in the plugin manager is still at 0.2. At the moment, new releases of plugins aren't automatically fetched, Geoff has to refresh the cache manually. I will ask him to do that (to make 0.3 available) as soon as we've got it working on your system.

[Ineluki80]

Ok, plugin works again, but Run... still doesn't work, even when using TurboMole coordinates.

The "Save results in" field and the "Run calculations in" field are still empty by default.
run.py : line 290 throws an error in copytree b/c "files are still in use".

If I use TurboMole coordinates and this code in run.py, at least everything works as it should:

       # If user specified a save location, copy calculation directory to there
        if avo_input["save_dir"] == "":                        # added this and the following line
           avo_input["save_dir"] = calc_dir
        if Path(avo_input["save_dir"]) != calc_dir:
            copytree(calc_dir, Path(avo_input["save_dir"]), dirs_exist_ok=True)

With this change in place, XYZ-coordinates still do not work, albeit the correct obabel version was found and sucessfully called, as can be seen in the log file. The input.xyz is still zero bytes.

Debug: OBProcess::executeObabel: Running "C:/Program Files/Avogadro2/bin/obabel.exe" "-icml -otmol" (:0, )

Warning: "C:/Program Files/Avogadro2/bin/obabel.exe"  stderr:
 "1 molecule converted\r\n" (:0, )

Warning: ("Parse error at offset 1: 'illegal value'\nRaw JSON:\n\nTraceback (most recent call last):\r\n  File \"C:\\Users\\USER\\AppData\\Local\\OpenChemistry\\Avogadro\\commands\\avo_xtb-main\\run.py\", line 240, in <module>\r\n    geom_cjson = json.load(result_cjson)\r\n                 ^^^^^^^^^^^^^^^^^^^^^^^\r\n  File \"C:\\Program Files\\Python\\312\\Lib\\json\\__init__.py\", line 293, in load\r\n    return loads(fp.read(),\r\n           ^^^^^^^^^^^^^^^^\r\n  File \"C:\\Program Files\\Python\\312\\Lib\\json\\__init__.py\", line 346, in loads\r\n    return _default_decoder.decode(s)\r\n           ^^^^^^^^^^^^^^^^^^^^^^^^^^\r\n  File \"C:\\Program Files\\Python\\312\\Lib\\json\\decoder.py\", line 337, in decode\r\n    obj, end = self.raw_decode(s, idx=_w(s, 0).end())\r\n               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\r\n  File \"C:\\Program Files\\Python\\312\\Lib\\json\\decoder.py\", line 355, in raw_decode\r\n    raise JSONDecodeError(\"Expecting value\", s, err.value) from None\r\njson.decoder.JSONDecodeError: Expecting value: line 1 column 1 (char 0)\r\n") (:0, )

I checked the command in convert.py : line 58, but it seems to be correct:
[WindowsPath('C:/Program Files/Avogadro2/bin/obabel.exe'), '-i', 'tmol', WindowsPath('C:/Users/USER/AppData/Local/OpenChemistry/Avogadro/commands/avo_xtb-main/last/input.tmol'), '-o', 'xyz', '-O', WindowsPath('C:/Users/USER/AppData/Local/OpenChemistry/Avogadro/commands/avo_xtb-main/last/input.xyz')]

[matterhorn103]

Ok, plugin works again, but Run... still doesn't work, even when using TurboMole coordinates.

1. What about if you don't use tmol? That cuts out the first conversion step and makes things a little simpler.

2. Do other calculation types run fine? Normal opt, freq etc.?

3. Can you show me the contents of your config.json file?

The "Save results in:" field gets, at least in theory, populated by Avogadro with the value of calc_dir, so your suggested code either isn't necessary, or it doesn't fix the real problem. The question is: why is the field empty.

You shouldn't start the calculation from the Run... window without making sure that field is populated with some directory.

I checked the command in convert.py : line 58, but it seems to be correct:

4. What if you run the command yourself from the command line? Can you convert the tmol to xyz by calling obabel manually?

[Ineluki80]

Sorry for the late reply. Last days were very busy.

1. Run doesn't work regardless of the tmol step.

2. Yes, other calculations like opt and freq work fine.

3. The content of my config.json is as follows:
   {"solvent": null, "energy_units": "kJ/mol", "calc_dir": "C:\\Users\\USER\\AppData\\Local\\OpenChemistry\\Avogadro\\commands\\avo_xtb-main\\last", "xtb_bin": "C:\\Users\\USER\\AppData\\Local\\OpenChemistry\\Avogadro\\commands\\avo_xtb-main\\xtb\\bin\\xtb"}

4. From the command line, it says "0 molecules converted". I'll dig into the reason.

[matterhorn103]

Thanks, and no worries, it's not very time critical.

On the basis of 2 and 4, it looks like the problem is conversion to and from .tmol with Open Babel. With Run, this is always needed, either .tmol > .xyz pre-calculation if the use of tmol is turned off, or .tmol > .cjson post-calculation if tmol is turned on.

The thing is it kind of has to be this way in order to allow for periodic systems.

I'll see if I can implement manual conversion of cjson to xyz so that Open Babel isn't needed if tmol wasn't turned on. It will still be broken if tmol is turned on, but at least it would be half of the problem solved.

[Ineluki80]

OK, I found the culprit. It's the input.tmol file. I had a look at it with a hex editor.
Instead of normal line endings like "\r\n" for Windows, "\n" for Unix or "\r" for Mac it uses "\r\r\n".
Therefore obabel treates it as Mac line endings and interprets it as interlaced with empty lines like this:

$coord

   -8.18901477909222      1.57739219105465     -0.00000600125995      c

   -9.90135231273266      1.79569713246616     -1.05297618383380      h

   -8.15004862639485      2.92197012351009      1.50966440673488      h

   -8.09987639777627     -0.29365966037175      0.76139522219954      h

   -6.60478177946510      1.88555927888798     -1.21810612181412      c

   -6.69392016078104      3.75660546113601     -1.97950701319203      h

   -4.89244424582465      1.66725433747647     -0.16513655202175      c

   -6.64374793216246      0.54098134643254     -2.72778186690575      h

   -4.85347809312729      3.01182660075353      1.34453257902844      h

   -4.80330586450871     -0.20379751394993      0.59626339449310      h

   -3.30821124619753      1.97542520476205     -1.38323794952056      h

$end

Removing the empty lines manually solves the conversion problem.

[matterhorn103]

Always those pesky newlines! That's very useful, well spotted.

Quite odd though. It's not just saving the file with the wrong line endings, something is adding extra ones. Critically, I don't get why that should only apply to tmol and not other formats – I'll raise that with Geoff.

Related to that, would you do me one more favour? When you run e.g. an optimization, what are the line endings in the input.xyz file?

I suspect the problem arises when the tmol file is passed as a string from Avogadro to the plugin, as the file is passed as a string and I guess uses your native line endings within the string, i.e. \r\n. In Python internally \n is the newline escape sequence regardless of platform, so Python sees it as \r<newline>. Python then saves the file, and when saving files replaces \n with whatever the native line ending is, in your case \r\n, so the result is \r\r\n.

I can think of two simple fixes, either to remove any \r characters as a first step, or to write in binary mode rather than text mode. I suspect the first makes more sense, I'll write that now.

By the way, macOS also uses \n like Unix actually, ever since OS X. It did use \r, but a long time ago now.

[Ineluki80]

Related to that, would you do me one more favour? When you run e.g. an optimization, what are the line endings in the input.xyz file?

Line Endings are "\r\n", as expected.

[matterhorn103]

Yeah ok, looks like Avogadro is inconsistent then and I guess maybe it's to do with whether Avogadro uses Open Babel to prepare the input geometry or not. I'll raise it on the forum.

The commits I made yesterday should hopefully fix the problems you describe here? If they do I'll close this issue.

[Ineluki80]

I tried run using tmol format.

1. Problem

avo_xtb-main\run_calc.py, line 131, in <module> tmol_file.write()
TypeError: TextIOWrapper.write() takes exactly one argument (0 given)

I assume, it's tmol_file.write(tmol_str) instead of tmol_file.write().

2. Problem
   Still had to add this code in run_calc.py:227 to make it work

if avo_input["save_dir"] == "":
           avo_input["save_dir"] = calc_dir

After these two fixes, run works with tmol format.

I tried run without tmol format.

I get the following error message because the last/input.xyz is empty - 0 bytes. (Also it's the only file in last/.)

avo_xtb-main\\run_calc.py\", line 139, in <module> xyz_file.write("\n".join(xyz))
TypeError: sequence item 0: expected str instance, int found

The complete log since running the avo_xtb plugin is:

Warning: "Unhandled widget in collectOptions for option 'help'." (:0, )

Debug:  writing to  cml (:0, )

Debug: OBProcess::executeObabel: Running "C:/Program Files/Avogadro2/bin/obabel.exe" "-icml -otmol" (:0, )

Warning: "C:/Program Files/Avogadro2/bin/obabel.exe"  stderr:
 "1 molecule converted\r\n" (:0, )

Warning: ("Parse error at offset 1: 'illegal value'\nRaw JSON:\n\nTraceback (most recent call last):\r\n  File \"C:\\Users\\USER\\AppData\\Local\\OpenChemistry\\Avogadro\\commands\\avo_xtb-main\\run_calc.py\", line 139, in <module>\r\n    xyz_file.write(\"\\n\".join(xyz))\r\n                   ^^^^^^^^^^^^^^\r\nTypeError: sequence item 0: expected str instance, int found\r\n") (:0, )

[matterhorn103]

I assume, it's tmol_file.write(tmol_str) instead of tmol_file.write().

Yep, that was a stupid error, fixed it.

Still had to add this code in run_calc.py:227 to make it work

Ok, good that it works with that fix. That's a bug with Avogadro though so I opened an issue for it. I don't think it makes sense to implement a hot-fix for it – it leads to inconsistent behaviour from the user's point of view. The user-facing behaviour would be the same even if I added the fix i.e. the field is empty, so the user won't have any idea that the fix is in place, it will still appear broken.

I'll see if I can fix the issue in Avogadro though. I already solved the same problem once. Interestingly my own build of Avogadro doesn't suffer the same issue. Are you running the nightly build of Avo?

I get the following error message because the last/input.xyz is empty - 0 bytes.

Fixed.

I checked with opt and run, with tmol and xyz, and everything works fine for me now. Let me know if it does for you too (when you add your fix at run_calc.py:227).

[Ineluki80]

Works beautifully now. With and without tmol coordinates.

Interestingly my own build of Avogadro doesn't suffer the same issue. Are you running the nightly build of Avo?

I am running today's Nightly.

Ok, good that it works with that fix. That's a bug with Avogadro though so I opened an issue for it. I don't think it makes sense to implement a hot-fix for it – it leads to inconsistent behaviour from the user's point of view. The user-facing behaviour would be the same even if I added the fix i.e. the field is empty, so the user won't have any idea that the fix is in place, it will still appear broken.

I'm not sure I get what you are saying. ATM, the bug in Avogadro is that it gobbles up the save path, so the field is empty. Indeed, this should be fixed in Avogadro itself. But imagine, a user is not interested anymore in using their own path and wants to empty the field. Should they be expected to look up what the default directory is and enter it manually ? Should the script fail without warning if the field is empty ? IMHO the most sensible meaning of an empty path should be the default directory for computation. That the script is then also working despite Avogadro's bug is a bonus. In no case should we try to copy to a directory based on an empty string. That's like relying on undefined behaviour.

[matterhorn103]

I'm glad that it works well now. Since the original issue was solved, I'll close this.

I'm not sure I get what you are saying.

Having thought about it somewhat I think I do agree with you, but I think for different reasons. Just to clarify what's going on:

Every calculation is always run, and the results are generated, in calc_dir, which is set via the plugin's "Configure..." dialog. This happens regardless of what the "Save results in:" field contains. But that gets overwritten for every new calculation, so I wanted to provide the user with a way to choose a convenient save location. If the user provides a path other than calc_dir, the results are copied in full at the end of the calculation.

The copying doesn't need to happen unless the user asked for it. So if anything, what should happen if that field is an empty string, is the same as if it is left as the calc_dir - that nothing gets copied. So although that is actually what your suggested fix does, it confused me as it is unnecessary to set avo_input["save_dir"] to anything if the user doesn't want a copy. Besides which, I want to treat avo_input as read-only, it should always retain the original values received from Avogadro. What I will do though is:

if not (avo_input["save_dir"] in ["", None] or Path(avo_input["save_dir"]) == calc_dir):
    copytree(calc_dir, Path(avo_input["save_dir"]), dirs_exist_ok=True)

But imagine, a user is not interested anymore in using their own path and wants to empty the field.

The idea was that they should leave it as the calc_dir, which is what fills that field every time the "Run..." dialog is opened.

But what I might do is change things a little to clarify what "Save results in:" means, since it seems to be confusing. Possibly I'll reword the description to make it's clear that it's only for an extra copy, and set the default value of the field to be blank anyway.