SwissDataScienceCenter / contributed-project-templates

A repository of community-developed project templates.
Apache License 2.0
4 stars 15 forks source link

update aiida project template #4

Closed ltalirz closed 3 years ago

ltalirz commented 3 years ago

Updates to the AiiDA project template

Questions:

Todo:

ltalirz commented 3 years ago

Example to test:

Repository URL: https://github.com/ltalirz/contributed-project-templates Archive URL: https://archive.materialscloud.org/record/file?record_id=638&filename=SSSP_1.1_PBE_efficiency.aiida&file_id=2a59c9e7-9752-47a8-8f0e-79bcdb06842c

image

rokroskar commented 3 years ago

Hi @ltalirz thanks for this update! Regarding your first question have you tried adding an export VAR=val to the post-init script?

ltalirz commented 3 years ago

Thanks - for some reason I didn't think of trying this.

I've modified the post-init.sh here: https://renkulab.io/gitlab/leopold.talirz1/mc-import4/-/blob/master/post-init.sh#L39

But it turns out that doesn't propagate to the environment in the terminal image

(same in the jupyter notebook)

rokroskar commented 3 years ago

Ah you're right of course - I thought the script was sourced but it's actually executed with bash. I think if we sourced it instead it would (probably) work...

ltalirz commented 3 years ago

It would be quite useful to be able to set environment variables, so I would support this.

rokroskar commented 3 years ago

Ok I will make an issue for it - I won't be able to get to it today, most likely...

ltalirz commented 3 years ago

@rokroskar this should be ready for review now. I tested it and it seems to work fine.

P.S. By the way, I again get the warning about an outdated renku version, but it displays the same version for the new one and the one of the repo image

ltalirz commented 3 years ago

thanks for the review @lorenzo-cavazzi !

I tried to run the code on the notebook but I got an error,

Would you mind sharing the error you saw? I remember trying it and it did work fine for me.

It may have been that you did not specify a path to an AiiDA archive file (although that field was optional, part of the template did assume that a valid URL was provided). For an example URL see https://github.com/SwissDataScienceCenter/contributed-project-templates/pull/4#issuecomment-818966293

The archive URL will be prefilled for users that enter via the Materials Cloud Archive, but of course the template should still work if someone creates a project from it manually and does not specify the url. I've now put in some extra guards to also cover that use case.

ltalirz commented 3 years ago

P.S. Yes, it is expected that the daemon is not running - I forgot to update the expected output in the README (now fixed).

lorenzo-cavazzi commented 3 years ago

After your commits, I don't get the exception anymore. It's probably not relevant, but here is the error I got before:

Screenshot_20210504_140413

Currently, if I don't provide the initial URL, it fails gracefully :+1:

Screenshot_20210504_140435

There is another minor detail on the README.md to possibly adjust, I'll provide the details in a commit suggestion. Fell free to include or discard it, then we are ready to merge this PR.

I have one final question: I noticed the repository contains uncommitted changes when I access a pristine environment. I assume a post initialization script runs something that creates those files. Would It make sense to exclude the repo folder using the .gitignore file? Or adding some logic to the script to run also a git commit when the new files are created? I'm not familiar with this system, so disregard my comment if the current behavior is the expected one.

ltalirz commented 3 years ago

I have one final question: I noticed the repository contains uncommitted changes when I access a pristine environment. I assume a post initialization script runs something that creates those files. Would It make sense to exclude the repo folder using the .gitignore file? Or adding some logic to the script to run also a git commit when the new files are created?

Thanks! The repo folder contains the internal provenance tracking of AiiDA which I would not suggest to commit to git for the time being (for this one should think about a deeper integration between renku and AiiDA first) - however, I would prefer if users can restore it when they come back to the environment.

Will adding it to the .gitignore also lead to the contents of the folder being lost when the environment is shut down? If yes, I suggest to keep it as is.

lorenzo-cavazzi commented 3 years ago

The code won't be preserved when starting a new environment from a new commit. I personally believe it's very easy that users may add that folder by mistake to a commit if it's not ignored, but I have nothing against leaving it as is if you think it's fine. That can also be changed later if needed.

ltalirz commented 3 years ago

Ok!

I'm sure there will be further changes once people start using it, and we'll try to make it as user-friendly as possible.

lorenzo-cavazzi commented 3 years ago

I've tagged a new version here and included the reference in the main renku repository, so that the new templates will be included in renkulab.io on the next release SwissDataScienceCenter/renku#2114 .

If this is urgent, let us know and we can speed up the process