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

BigDFT: common relax workchain excepts because of missing pseudo potential file #158

Closed
sphuber opened this issue Mar 10, 2021 · 8 comments · Fixed by #169
Closed

BigDFT: common relax workchain excepts because of missing pseudo potential file #158

sphuber opened this issue Mar 10, 2021 · 8 comments · Fixed by #169
Assignees
Labels
type/bug Something isn't working

Comments

@sphuber
Copy link
Collaborator

sphuber commented Mar 10, 2021

Excepts in presubmit:

ValueError: path `/.virtualenvs/aiida/lib/python3.7/site-packages/BigDFT/scripts/psppar/psppar.Si` does not correspond to an existing file

This is on Quantum Mobile with aiida-bigdft==0.2.5.
Are these files included in the MANIFEST.in @adegomme ? This is necessary for them to be included in the distribution that is released on PyPI.

@sphuber sphuber added the type/bug Something isn't working label Mar 10, 2021
@adegomme
Copy link
Collaborator

adegomme commented Mar 10, 2021

I can confirm that the files are in the pip package, but I wonder if the path is not missing an aiida-bigdft before BigDFT .. is /.virtualenvs/aiida/lib/python3.7/site-packages/aiida_bigdft/BigDFT/scripts/psppar/psppar.Si a file ?

@sphuber
Copy link
Collaborator Author

sphuber commented Mar 10, 2021

Actually, /home/max/.virtualenvs/aiida/lib/python3.7/site-packages/BigDFT/scripts/psppar/psppar.Si does exist. It seems that the $HOME expansion may be failing somehow because it is missing /home/max in the path. I wonder what part of the code is responsible for creating the entire path though? Don't think the plugin should have anything to do with it, but then who?

@sphuber
Copy link
Collaborator Author

sphuber commented Mar 10, 2021

Ah, I see now. The pseudos are in the BigDFT package itself, and not the Python one. This is probably where the problem lies somewhere. I was looking at the BigDFTCalculation and saw the following:

        # setup pseudopotentials if needed
        if self.inputs.pseudos is not None:
            for filename in self.inputs.pseudos:
                pseudo_filedata = SinglefileData(
                    file=os.path.abspath(filename)).store()
                local_copy_list.append(
                    (pseudo_filedata.uuid, pseudo_filedata.filename, pseudo_filedata.filename))

and that the pseudos are past in the inputs as a List node with strings that are the full path. May I ask why you chose this approach? This is very fragile (and I think has to do with the error I describe in the OP) and not the recommended way in AiiDA I think.

The problem actually lies in the input generator implementation. It uses os.rel to build the filename paths of the pseudos:

but they can then be relative as shown by the attributes of the pseudos input List node:

(aiida) max@a54d4363604c:/$ verdi node attributes 4092
PK: 4092
{
    "list": [
        "../.virtualenvs/aiida/lib/python3.7/site-packages/BigDFT/scripts/psppar/psppar.Si"
    ]
}

I think we can fix the immediate problem by calling os.path.normpath on the relative paths, turning them into absolute paths. However, I would still recommend to rethink this design of pseudos at some point and maybe consider the approach that other plugins use where the pseudos input is a namespace and the pseudo potentials that should be used are passed in as nodes ( SinglefileData as the most simple case) at runtime.

@sphuber
Copy link
Collaborator Author

sphuber commented Mar 10, 2021

Can confirm that normalizing the path in the input generator fixes the immediate problem, although there is another exception now. This one that comes from the LogFile.

*** 1 LOG MESSAGES:
+-> REPORT at 2021-03-10 12:09:31.811655+00:00
 | [4754|BigDFTCalculation|on_except]: Traceback (most recent call last):
 |   File "/home/max/.virtualenvs/aiida/lib/python3.7/site-packages/plumpy/process_states.py", line 225, in execute
 |     result = self.run_fn(*self.args, **self.kwargs)
 |   File "/home/max/codes/aiida-core/aiida/engine/processes/calcjobs/calcjob.py", line 313, in parse
 |     exit_code_retrieved = self.parse_retrieved_output(retrieved_temporary_folder)
 |   File "/home/max/codes/aiida-core/aiida/engine/processes/calcjobs/calcjob.py", line 392, in parse_retrieved_output
 |     exit_code = parser.parse(**parse_kwargs)
 |   File "/home/max/.virtualenvs/aiida/lib/python3.7/site-packages/aiida_bigdft/parsers.py", line 63, in parse
 |     get_abs_path(output_filename))
 |   File "/home/max/.virtualenvs/aiida/lib/python3.7/site-packages/aiida_bigdft/data/__init__.py", line 116, in __init__
 |     self.logfile = path
 |   File "/home/max/.virtualenvs/aiida/lib/python3.7/site-packages/aiida_bigdft/data/__init__.py", line 132, in logfile
 |     self.bigdftlogfile = Logfiles.Logfile(path)
 |   File "/home/max/.virtualenvs/aiida/lib/python3.7/site-packages/BigDFT/Logfiles.py", line 392, in __init__
 |     raise ValueError("No log information provided.")
 | ValueError: No log information provided.

@adegomme
Copy link
Collaborator

adegomme commented Mar 10, 2021

Thanks a lot for the analysis, I will have a look this afternoon and push a fix asap. I won't be able to attend the meeting though.

@adegomme
Copy link
Collaborator

I was trying to reproduce on a quantum mobile docker with 0.2.5 yesterday, but failed to trigger it using cli, what script/command did you use (maybe it was on the vm but I can't have it built, hostname issues are blocking) ?
My laptop does not have enough ram to handle a full computation to reach the end, I have to try on a proper machine for the second issue.. (And that means rebuilding, as apparently docker save/scp/import is useless and loses important information to run containers in the process)

@sphuber
Copy link
Collaborator Author

sphuber commented Mar 11, 2021

I added a full script in this issue. It includes the docker-compose.yml script that I used and how I run the workflow through the CLI. Let me know if that works for you.

@adegomme
Copy link
Collaborator

adegomme commented Mar 11, 2021

Thanks, indeed I can now reproduce on my docker build, if the command line is run from / (as done in your script, the only change I made on my attempts yesterday was to move to /home/max first). I will now check how to avoid this.

adegomme added a commit to adegomme/aiida-common-workflows that referenced this issue Mar 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants