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

Add AiiDA base example notebook #40

Merged
merged 4 commits into from
Nov 7, 2024
Merged

Conversation

mbercx
Copy link
Collaborator

@mbercx mbercx commented Oct 10, 2024

Add a very basic implementation of the EOS work chain using aiida-shell. Missing some provenance features, but already has parsing into nodes implemented for the Quantum ESPRESSO calculation.

Copy link
Contributor

@GeigerJ2 GeigerJ2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Difficult to review on GH directly as the notebook is shown in raw JSON :D But a few comments:

  • Overall, is very nice now, very simple (despite missing some features, like full provenance, but we are not aiming for that here anyway)
  • Maybe provide a link to the aiida-shell repo?
  • Just as a reminder that the write_input function currently in functions.py (which will eventually end up in a shared directory to be imported into all notebooks, I suppose?) should also contain the extra return_string argument
  • I would maybe add some additional text introducing AiiDA concepts, such as Data nodes, as is done for jobflow, and then comment on why, e.g., SinglefileData.from_string is used in the call to launch_shell_job. Though, I guess this depends on how verbose the notebooks should finally be. I can add a commit on top of this PR, if that's OK, @mbercx?
  • Also, happy to add some text to the remaining entries!

Comment on lines +27 to +33
def append_to_list(lst: list, item: float):
lst.append(item)


def split_string(string: str, character: str) -> list:
return string.split(character)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
def append_to_list(lst: list, item: float):
lst.append(item)
def split_string(string: str, character: str) -> list:
return string.split(character)

@jan-janssen
Copy link
Member

It would be great if you could also add the notebook to the continuous integration environment. The previous aiida notebook is tested automatically, so it should be possible to adjust the GitHub CI to test the new notebook. In addition, you can remove the old notebook, so we do not confuse the readers.

@jan-janssen jan-janssen merged commit 4d25b36 into main Nov 7, 2024
5 checks passed
@jan-janssen jan-janssen deleted the fix/aiida-shell-version branch November 7, 2024 15:02
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

Successfully merging this pull request may close these issues.

3 participants