HECBioSim/Longbow

LAMMPs included scripts and pair coefficients not transferred

anjohan opened this issue · 7 comments

While longbow transfers data files used by read_restart etc. perfectly, it seems unable to transfer included scripts and potential files.

Example commands:

  • include variables.in
  • pair_coeff * * SiO2.vashishta Si O

I attempted to fix this by adding the include keyword here, but it didn't work.

Hi Anders,

Many thanks for reporting this bug, and thanks also for your other fix. Firstly, my apologies for my inactivity on this, I have been at a conference today so have not had chance to look at this. I will be back at my office tomorrow and will issue a fix for this, and will merge in your other pull request to the new version.

Thank you very much!

(Also: If one day without merging pull requests and fixing edge-case bugs is 'inactivity', I like your workflow ;-) )

I attempted to fix this by adding the include keyword here, but it didn't work.

Doing this should have allowed you to have detected files supplied with the "include" keyword as you said. I'm not sure why that didn't work for you, I have just tried the same and wrote a unit test for it which seems to show that the include file is now being picked up.

This does not however resolve the "SiO2.vashishta" part of the pair_coeff argument, I'm assuming that SiO2.vashishta is also a file that should be transferred by Longbow? I will probably need to use some better regex to detect such files, I will work on something for this now.

Doing this should have allowed you to have detected files supplied with the "include" keyword as you said. I'm not sure why that didn't work for you

I've been having some problems with different installations interfering, which may have been the cause.

(It seems it would have been smarter of me to do pip install . rather than python setup.py install, as pip can both uninstall and force a refresh.)

This does not however resolve the "SiO2.vashishta" part of the pair_coeff argument, I'm assuming that SiO2.vashishta is also a file that should be transferred by Longbow?

Correct, that needs to be transferred. The filename of the potential is always the third argument (fourth word) in the line, but the third argument is not always a filename.

I guess one simple, if not necessarily elegant, way would be to check if any of the words in the file match a filename, and is not part of an output command.

From scanning through the 10 pages of search results returned when searching for 'file' in the LAMMPs documentation, I find the output commands
print, dump, log, restart, write_dump, write_restart, write_data, pair_write, write_coeff, ave/time, ave/chunk, bond_write, ave/histo, ave/histo/weight, saed/vtk.

Something like

do substitutions
files_to_be_transferred = []
output_commands = ["print", "dump", "log", "restart", "write_dump", "write_restart",
                   "pair_write", "write_coeff", "ave/time", "ave/chunk", "bond_write",
                   "ave/histo/weight", "saed/vtk", "ave/histo", "write_data"]
for line in file:
    words = line.split()
    if set(words).isdisjoint(output_commands): # Not an output command
        for word in words:
            if os.path.isfile(word):
                files_to_be_transferred.append(word)

(Apparently, set.isdisjoint is the fastest way to check for common elements in Python.)

One possible weakness (which might also apply to your current approach) is the possibility to split lines in LAMMPs by use of an ampersand, but I don't think it is very common to split an input/output command before the name of the input/output file.

I just tested running the script

import sys
import os
output_commands = [
    "print", "dump", "log", "restart", "write_dump", "write_restart",
    "pair_write", "write_coeff", "ave/time", "ave/chunk", "bond_write",
    "ave/histo/weight", "saed/vtk", "ave/histo", "write_data"
]
with open(sys.argv[1], "r") as infile:
    for line in infile:
        words = line.split()
        if set(words).isdisjoint(output_commands):  # Not an output command
            for word in words:
                if os.path.isfile(word):
                    print(word)

on some of my LAMMPs scripts and it seemed to work well (except I can't do substitutions, and I ignored the possibility of file names being commented out).

[Note that this is just an idea; I have no knowledge of whether this is compatible with the rest of longbow or not.]

Thanks for your code example, I will certainly try and use as much of it as I can! I will also add you to the list of contributors to Longbow, you are the first one. I should get this fixed by the end of today.

Yes pip is better at cleaning up after version changes, you just have to remember to do the local "pip install --local ." and not the "pip install --local longbow" for the development copies.

Hi Anders,

I have incorporated your code above into the LAMMPS plugin in a way that should preserve substitutions and recursive checking! The current unit test suite passes with this meaning no existing functionality is lost, however I don't think that they will fully test the limits of whether it will actually pick out the extra files so I will need to write some more tests to validate for this going forward. However, you should be able to test this for your case to see if it works. Let me know if it works for you.

Thanks again for pointing this out and for providing your own solution. I have added you to the contributors section of our AUTHORS file.