📚 DOCS: Add new tutorial modules - #7205
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #7205 +/- ##
==========================================
+ Coverage 80.67% 80.67% +0.01%
==========================================
Files 581 581
Lines 47002 47000 -2
==========================================
- Hits 37913 37912 -1
+ Misses 9089 9088 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
5930f90 to
d9ab1e4
Compare
| print(f"Fields shape: {data['U_final'].shape}") | ||
| ``` | ||
|
|
||
| ## Visualizing the results |
There was a problem hiding this comment.
THIS! Show this at the top (but maybe smaller, i.e. a diagram, like):
And a very easy, intuitive explanation of what I'm looking at. I have no idea at the moment, to be honest. Maybe I didn't read the content properly, but neither will users.
There was a problem hiding this comment.
As a side note: less inputs. Make some a default (probably seed?) Unless the are necessary for later (randomization?). Anyways, make the input file as small as possible for our needs.
There was a problem hiding this comment.
Also, maybe merge this section with the part that shows the results from the last one?
So we have
- Running the simulation
- Results
- ...
There was a problem hiding this comment.
Done! Added a static image of the UV fields right at the top of the module (below the intro text), so readers immediately see what the simulation produces. Merged the "Visualizing the results" section into "Running the simulation" (no separate heading). Removed Fields shape output. Re: reducing inputs, seed is already optional in the script (defaults to None), so the input file could drop it. Will revisit when simplifying further. Input file is anyway not shown atm. Could not be bothered to leave my beautiful terminal yet, to create a such a scheme, as you suggest, but we can do it when we also create the package!
|
Maybe |
khsrali
left a comment
khsrali
left a comment
There was a problem hiding this comment.
@GeigerJ2 @mbercx
Thanks a lot for the great work. One practical note to pay attention to is that, since you are executing rtd code snippets, then now this has to run on every PR.
Two issues:
-
At the moment the github action only triggers on PRs that they touch anything in docs/* otherwise the build is skipped. This can result in silent failure of your tutorial a PR might change something that your tutorial would break your codes in tutorial.
-
If you activate the entire rtd build for this reason, not a good idea, it's is very expensive. Better to execute them and make an output yml file out of them and unbound them from rtd build. But then the issue is where to store those yaml files? perhaps autocommit in a separate branch, that you is created only once for this purpose.
Alternatively and much easier: we could hand this to cloudflare workers if the long term plan is to merge aiida-core documentation in aiida.net anyways.
2a96a6d to
749b144
Compare
749b144 to
9c1da2d
Compare
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds a complete multi-module AiiDA tutorial series (modules 0-7 plus teaser) covering CLI execution, provenance tracking, calcfunctions, WorkGraph workflows, remote submission, querying, conditional/adaptive workflows, and further topics. It includes shared tutorial code, SLURM-based docs-build CI infrastructure, a Sphinx notebook post-processing extension, extensive internal planning/review notes, and a small IPython magic fix. Estimated code review effort: 4 (Complex) | ~75 minutes ChangesTutorial system and documentation infrastructure
Estimated code review effort: 4 (Complex) | ~75 minutes IPython %verdi magic variable expansion
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Readers work through these tutorials by copying the visible cells one by one, or by downloading the notebook, so any cell hidden with `remove-cell` / `remove-input` silently breaks them. The rendered page must be what actually runs, with nothing load-bearing hidden. Module 0 faked its run entirely: it showed a non-executable `$ gsrd input.yaml` console block while the cells that defined the working directory and ran the simulation were hidden, so copying the visible cells left `work_dir` undefined. It now runs everything in a visible `!mkdir -p tmp` scratch dir, keeps only the noisy `gsrd` output folded (`hide-output`), and adds a `pip install` note. The scratch dir is gitignored. Every module also hid its profile-setup cell and showed a `$ verdi presto` console block instead, so a copy-paster never ran `%load_ext aiida` and had no profile. The setup is now one visible cell everyone runs, framed as an isolated sandbox profile (`tutorial-<hash>`) that keeps a reader's data separate from their real work and reproduces on a fresh machine. The stale-profile cleanup moves into `setup_tutorial.py`; run from every module, it only ever removes older tutorial profiles, never the current one. Fix a latent bug this exposes: `setup_tutorial.py` called `get_config()`, which raises on a fresh `AIIDA_PATH` with no config yet (a new reader, or CI). Use `get_config(create=True)`. Local builds only worked because a real `~/.aiida` already existed. In Module 4, the container-specific plumbing (SSH config, poll interval, cleanup, transport check) is folded (`hide-input`) rather than removed, so a downloaded notebook shows it instead of silently carrying code that rewrites the reader's ~/.ssh/config.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (16)
docs/source/tutorials/module0.md-213-228 (1)
213-228: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDisplay stdout in the failure demonstration.
The cell prints only
result.stderr, but the explanation asks readers to observe the absence of*** JOB DONE ***on stdout. Print both streams or remove that claim so the behavior shown matches the text.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/source/tutorials/module0.md` around lines 213 - 228, Update the subprocess failure demonstration around result.returncode to also print result.stdout alongside result.stderr, preserving the existing exit-code output so readers can observe the missing “*** JOB DONE ***” marker described by the tutorial.docs/source/tutorials/module0.md-114-133 (1)
114-133: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winClose the
NpzFilehandle.
np.load()returns an open archive that remains alive across notebook cells. Repeated execution can leak file descriptors; use a context manager and retain only the arrays needed later.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/source/tutorials/module0.md` around lines 114 - 133, Update the results.npz loading code around the np.load call to use a context manager, ensuring the NpzFile handle closes after reading. Retain the arrays needed by later cells, including params, rather than accessing data after the context exits.docs/source/tutorials/module2.md-371-373 (1)
371-373: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDo not label parser nodes as CalcJobs.
r['parsed']['variance_V'].creatoris theparse_outputcalcfunction node that created theFloat, not the underlyingShellJob. Rename the variables and prose to describe parser nodes, or explicitly traverse to the ShellJob before tagging and grouping.Also applies to: 395-397
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/source/tutorials/module2.md` around lines 371 - 373, Update the tutorial’s variables and surrounding prose to consistently identify r['parsed']['variance_V'].creator as the parse_output parser CalcFunctionNode, not a CalcJob or underlying ShellJob. Apply the same terminology correction to the related content around lines 395-397; if ShellJob-specific tagging or grouping is intended, explicitly traverse to that node before applying it.docs/source/tutorials/module1.md-292-305 (1)
292-305: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winAvoid requiring
treein the tutorial exampledocs/source/tutorials/module1.md:292-305— use a PythonPath.rglob()listing here, or addtreeto the prerequisites; the current step can fail in a standard AiiDA environment.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/source/tutorials/module1.md` around lines 292 - 305, Replace the shell-based tree command in the tutorial’s dump-directory example with a Python pathlib listing using Path.rglob(), while preserving the demonstration of the dumped directory contents and avoiding any external tree utility dependency.docs/source/tutorials/module4.md-287-290 (1)
287-290: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winClarify the remote prerequisites.
AiiDA itself need not be installed on the cluster, but this example still requires an SSH server, SLURM, and the
gsrdexecutable at/opt/gsrd/bin/gsrd. Rephrase this to avoid implying that the HPC host requires no software or services.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/source/tutorials/module4.md` around lines 287 - 290, Revise the tip in the module 4 tutorial to clarify that AiiDA is not installed on the HPC, while the remote host still requires an SSH server, SLURM, and the gsrd executable at /opt/gsrd/bin/gsrd. Remove the blanket claim that nothing needs to be installed on the HPC, while preserving the explanation that no sudo rights are required.docs/source/tutorials/module5.md-302-304 (1)
302-304: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAvoid overstating database complexity guarantees.
count(),SUM, and top-N queries are not generally O(1) or O(log N); their complexity depends on indexes, selectivity, query plans, and whether the database can use an index-only strategy. Describe these as potentially sublinear with suitable indexes rather than making a general guarantee.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/source/tutorials/module5.md` around lines 302 - 304, Revise the “Python loop vs SQL” discussion to avoid guaranteeing O(1) or O(log N) complexity for count(), SUM, and top-N queries. State that these operations can be sublinear when suitable indexes and query plans apply, while acknowledging complexity depends on factors such as selectivity and index-only execution; retain the contrast that the example’s two paths both visit every row.debug-module4.md-165-166 (1)
165-166: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winFix the cleanup sequence for AiiDA provenance.
After the example creates calculations and remote data,
verdi computer delete slurm-sshwill be rejected while nodes still reference that computer. Instruct users to remove the generated nodes or discard the disposable tutorial profile instead. AiiDA prevents deleting computers with linked calculation or remote-data nodes. (aiida.readthedocs.io)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@debug-module4.md` around lines 165 - 166, Update the cleanup instructions near “Remove AiiDA computer” to first remove the generated calculation and remote-data nodes, or explicitly instruct users to discard the disposable tutorial profile, before running “verdi computer delete slurm-ssh”. Ensure the sequence does not attempt to delete a computer while linked provenance nodes remain.debug-module4.md-119-126 (1)
119-126: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAnchor the input path to the repository root.
Path('docs/source/tutorials/include/input.yaml').resolve()only works when the command is launched from the repository root. Make the guide explicitlycdto the root first or derive the path from a known repository-root variable.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@debug-module4.md` around lines 119 - 126, Update the launch example around input_path and launch_shell_job so the input YAML path is resolved relative to a known repository root, or explicitly change into that root before resolving it. Preserve the existing input_path value and job-launch behavior while making the guide independent of the caller’s current working directory.docs/source/tutorials/include/setup_tutorial.py-93-100 (1)
93-100: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winNo warning logged if the broker never comes up within the 10s deadline.
The loop silently exits after the deadline even if
_broker.is_runningis stillFalse, so a startup failure here surfaces later as a confusingverdi status/submission failure rather than a clear message at setup time.🛡️ Proposed fix: warn on timeout
if isinstance(_broker, ZmqBroker): _deadline = time.monotonic() + 10.0 while not _broker.is_running and time.monotonic() < _deadline: time.sleep(0.2) + if not _broker.is_running: + print('WARNING: ZMQ broker did not report running within 10s; async submission may fail.')🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/source/tutorials/include/setup_tutorial.py` around lines 93 - 100, Update the broker startup wait around _broker.is_running to detect when the 10-second deadline expires while the broker remains stopped, and log a warning with clear startup-timeout context before continuing. Preserve the existing polling behavior and avoid warning when the broker becomes running within the deadline.docs/source/tutorials/TRACKING.md-43-43 (1)
43-43: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winResolve the Module 1 → Module 6 checklist dependency.
Module 1 handling failures remains unchecked, while Module 6 says its error-handler content depends on it and Module 7 is already marked complete. Either finish/check off the prerequisite or update the dependency wording so the checklist reflects the actual release state.
Also applies to: 150-150, 164-164
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/source/tutorials/TRACKING.md` at line 43, Resolve the unchecked Module 1 “Handling failures” item in TRACKING.md by either completing and checking it off, or revising the Module 6 dependency wording to reflect the actual release state; apply the same correction to the additional referenced checklist entries and keep the Module 7 completion status consistent.docs/source/tutorials/_notes/asyncssh-issue-draft.md-76-76 (1)
76-76: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winFill in the reproduction environment before posting.
Replace
<insert>with the exact AsyncSSH version, Python version, and operating system so maintainers can reproduce the report.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/source/tutorials/_notes/asyncssh-issue-draft.md` at line 76, Update the reproduction environment note in the AsyncSSH issue draft by replacing each <insert> placeholder with the exact AsyncSSH version, Python version, and operating system used. Preserve the existing report format and ensure no placeholders remain.docs/source/tutorials/_notes/module4-local-debug.md-102-106 (1)
102-106: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winProvide a safe way to restore the global configuration.
get_config().set_option(...)persists the changed transport settings beyond this debugging session, but the guide only says to reset them. Capture the previous values and restore them, or provide exact reset commands, so subsequent AiiDA operations are not affected.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/source/tutorials/_notes/module4-local-debug.md` around lines 102 - 106, Update the tutorial’s AiiDA configuration example around get_config().set_option to provide an explicit restoration path: capture the original values before changing transport.task_retry_initial_interval and transport.task_maximum_attempts, then restore those values after debugging, or include exact commands using the documented defaults. Ensure subsequent AiiDA operations are not affected by the temporary settings.docs/source/tutorials/_notes/api-discrepancies.md-169-177 (1)
169-177: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winSynchronize the module numbering with the current tutorial plan.
This table places WorkGraph in Module 4 and querying/error handling in later modules, while
docs/source/tutorials/TRACKING.mdcurrently defines Module 3 as WorkGraph, Module 4 as remote submission, Module 5 as querying, and Module 7 as “Where to go next.” Update this table or mark it explicitly as a historical snapshot to prevent contributors from following the wrong checklist.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/source/tutorials/_notes/api-discrepancies.md` around lines 169 - 177, Synchronize the module labels and discrepancy entries in the table with the current tutorial plan defined in TRACKING.md, including WorkGraph as Module 3, remote submission as Module 4, querying as Module 5, and “Where to go next” as Module 7. Alternatively, explicitly label this table as a historical snapshot if its existing numbering must remain unchanged.docs/source/tutorials/_notes/module5-plan.md-27-30 (1)
27-30: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winExport via a group or explicit node IDs, not a raw QueryBuilder selection.
verdi archive createtakes nodes/groups (or identifiers); if the tutorial starts from a QB result, add a step to materialize it into a group before exporting.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/source/tutorials/_notes/module5-plan.md` around lines 27 - 30, Update the “Export & share” tutorial step to materialize QueryBuilder results into a group before invoking `verdi archive create`; describe export using that group or explicit node identifiers, not a raw QB selection, while preserving the existing light-touch scope.docs/source/tutorials/_notes/module7-plan.md-63-73 (1)
63-73: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winScope caching to the tutorial profile.
verdi config set caching.default_enabled Trueonly affects the current profile; if the tutorial switches profiles, enable it after selecting the tutorial profile or use a profile-local API so the cached second run can’t come from another profile.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/source/tutorials/_notes/module7-plan.md` around lines 63 - 73, Update the caching section’s enablement instructions so caching is explicitly configured after selecting the tutorial profile, or through a profile-local API. Ensure the ShellJob demonstration performs both runs within that same tutorial profile and cannot reuse a cache entry from another profile.docs/source/tutorials/_notes/session-tasks.md-11-17 (1)
11-17: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRemove the stale “open questions” status.
Line 11 says all four Module 7 questions are resolved, but line 17 still describes them as open. Update or label the historical planning entry so this tracker has one authoritative status.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/source/tutorials/_notes/session-tasks.md` around lines 11 - 17, Update the historical “M7 expanded plan” entry in the session-tasks tracker so its four questions are no longer presented as open; mark them as resolved or otherwise clearly label the entry as historical, while preserving the planned sections and context.
🧹 Nitpick comments (19)
docs/source/tutorials/module3.md (1)
223-231: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftAvoid teaching private WorkGraph internals as the normal API.
The tutorial relies on
_get_keys()and._value, and explicitly acknowledges that._valuemay change. Prefer public accessors; if the current dependency lacks them, isolate these calls in a compatibility helper and pin/document the supported aiida-workgraph version.Also applies to: 482-489, 593-600
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/source/tutorials/module3.md` around lines 223 - 231, Update the tutorial examples around the WorkGraph input/output listings to use public aiida-workgraph accessors instead of private `_get_keys()` and `._value`; if those accessors are unavailable, isolate the private calls in a compatibility helper, and pin/document the supported aiida-workgraph version. Apply the same change to the corresponding sections referenced by the comment.docs/source/tutorials/module6.md (1)
601-604: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid relying on the private
_valueattribute.These cells access
outputs.*._value, which is an implementation detail and can break across aiida-workgraph versions. Verify the supported public accessor for dynamic namespace outputs and use that instead.Also applies to: 616-618
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/source/tutorials/module6.md` around lines 601 - 604, Update the tutorial’s adaptive sweep output access in plot_adaptive_sweep and the additionally affected cells to use AiiDA WorkGraph’s supported public accessor for dynamic namespace outputs instead of the private _value attribute, preserving the existing coarse_variances and refined_variances values passed to the plotting function.debug-module4.md (1)
85-87: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winDocument the actual transport fallback.
This section says to fall back to OpenSSH, but it only runs
verdi computer configure core.ssh_async. Add the correspondingcore.sshcommand and explain when to use it, or remove the fallback claim. The AiiDA documentation treats these as distinct transport configurations. (aiida.readthedocs.io)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@debug-module4.md` around lines 85 - 87, Update the transport configuration section around the core.ssh_async command to document the distinct core.ssh OpenSSH configuration command and clearly state when to use it as the fallback. Ensure the documented commands match the claimed asyncssh-first, OpenSSH-fallback behavior..github/workflows/docs-build.yml (1)
68-110: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueLarge disabled workflow block committed in-tree.
This ~40-line commented-out future step is well-rationalized, but carrying disabled CI logic in the workflow file risks silent rot (referenced paths/branches drifting) before it's ever enabled. Consider tracking this as a linked issue/TODO instead of dead code in the workflow.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/docs-build.yml around lines 68 - 110, The disabled module4 baking workflow block should not remain as commented-out CI logic in .github/workflows/docs-build.yml. Remove the entire commented block, including its trigger and commit steps, and track the future implementation through a linked issue or concise TODO outside the workflow instead.docs/source/tutorials/include/tasks.py (2)
106-129: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated radial-FFT power-spectrum logic between
tasks.pyandplotting.py. Both files independently implement loadingV_final, centring, 2D FFT, and radial-averaging the power spectrum — one root cause (no shared helper), and the two copies have already drifted (see the divide-by-zero fix present only inplotting.py).
docs/source/tutorials/include/tasks.py#L106-L129: extract the shared load+FFT+radial-average logic (already flagged separately for its missing zero-guard) into a common helper.docs/source/tutorials/include/plotting.py#L154-L206: reuse the same shared helper here instead of re-implementing the FFT/radial-average steps inline.Consider adding a small shared function (e.g. in
constants.pyor a newfft.pyinclude module) that bothfft_peak_wavelengthandplot_fft_spectrumcall, so future fixes (like the zero-guard) don't need to be applied twice.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/source/tutorials/include/tasks.py` around lines 106 - 129, Extract the shared V_final loading, centring, 2D FFT, and radial power-spectrum calculation from fft_peak_wavelength in docs/source/tutorials/include/tasks.py:106-129 into a common helper, including the existing zero-radius guard. Update fft_peak_wavelength to call that helper, and update plot_fft_spectrum in docs/source/tutorials/include/plotting.py:154-206 to reuse it instead of duplicating the FFT and radial-averaging logic; both sites require changes.
32-36: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
prepare_input/bump_n_stepsdocstrings missing:param:tags.Unlike
parse_output,fft_peak_wavelength, andidentify_transition_regionin the same file, these two functions have only a one-line summary with no parameter documentation.
As per coding guidelines, "Use Sphinx-style docstrings (:param:,:return:,:raises:), with types written in annotations rather than docstrings."Also applies to: 132-137
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/source/tutorials/include/tasks.py` around lines 32 - 36, Update the docstrings for prepare_input and bump_n_steps to include Sphinx-style :param: documentation for each parameter, matching the existing documentation style and relying on type annotations for parameter types. Preserve their current summaries and behavior.Source: Coding guidelines
docs/source/tutorials/include/plotting.py (1)
34-46: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
plot_provenancedocstring missing:param:/:return:tags.Every other function in this file documents its parameters with Sphinx-style
:param:(and:return:/:returns:where relevant);plot_provenanceonly has a prose description fornodeand no return doc for theDigraphit produces.As per coding guidelines, "Use Sphinx-style docstrings (`:param:`, `:return:`, `:raises:`), with types written in annotations rather than docstrings."📝 Suggested fix
def plot_provenance(node: ProcessNode) -> Digraph: """Return a Graphviz digraph for *node* and its connected provenance. Traverses ancestors and descendants, including inputs/outputs of connected processes, so the full chain is visible. The graph renders as inline SVG in Jupyter notebooks. + + :param node: the process node to render provenance for. + :return: a Graphviz ``Digraph`` of the connected provenance subgraph. """🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/source/tutorials/include/plotting.py` around lines 34 - 46, Update the docstring of plot_provenance to document node with a Sphinx-style :param: entry and the returned Graphviz Digraph with a :return: entry, while keeping types in the existing annotations and preserving the current behavior.Source: Coding guidelines
docs/source/conf.py (1)
107-139: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider moving the future-work design note out of
conf.py.The ~20-line commented-out block describing the disabled pre-executed-notebook strategy for RTD is useful context but clutters the build configuration. Given the PR already ships a
_notes/directory andTRACKING.mdfor planning content, that design note would be more discoverable there, with a short one-line pointer left inconf.py.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/source/conf.py` around lines 107 - 139, The disabled pre-executed-notebook design note in conf.py should be moved to the project’s planning documentation, such as the existing _notes/ or TRACKING.md, while preserving its intent and trade-offs. Remove the multi-line commented block near nb_execution_excludepatterns and leave a concise one-line pointer to the relocated note.docs/source/tutorials/_notes/start-slurm-container.sh (1)
42-52: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOptional: guard against missing
uvon PATH.
which uvreturning empty would makereadlink -f ""resolve to an unexpected path rather than failing clearly. Since this is a local dev convenience script (excluded from CI/build), low priority.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/source/tutorials/_notes/start-slurm-container.sh` around lines 42 - 52, In the uv setup block, validate that the `uv` executable lookup succeeds before calling `readlink -f` or `docker cp`. Update the `UV_HOST` initialization around `which uv` to fail clearly with an actionable message when uv is absent, while preserving the existing copy and container installation flow when it is found.docs/source/_ext/inline_downloads.py (1)
61-270: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrivate helper docstrings lack Sphinx-style
:param:/:return:fields. Coding guidelines require Sphinx-style docstrings with types in annotations rather than docstrings; the helpers below use plain one-line docstrings only.
docs/source/_ext/inline_downloads.py#L61-L270: add:param:/:return:fields to_inline_run_cells,_read_include,_convert_myst_block,_inline_literalinclude,_convert_inline_roles,_clean_markdown, and_process_markdown_cells.docs/source/tutorials/include/setup_slurm.py#L65-L93: add:param:/:return:fields to_container_reachable,_strip_legacy_block, and_strip_current_block.As per coding guidelines, "Use Sphinx-style docstrings (
:param:,:return:,:raises:), with types written in annotations rather than docstrings."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/source/_ext/inline_downloads.py` around lines 61 - 270, Add Sphinx-style :param: and :return: fields to the docstrings of _inline_run_cells, _read_include, _convert_myst_block, _inline_literalinclude, _convert_inline_roles, _clean_markdown, and _process_markdown_cells in docs/source/_ext/inline_downloads.py (lines 61-270), using existing annotations for types. Apply the same documentation update to _container_reachable, _strip_legacy_block, and _strip_current_block in docs/source/tutorials/include/setup_slurm.py (lines 65-93), adding :param: and :return: fields without duplicating type information in the docstrings.Source: Coding guidelines
docs/source/tutorials/_notes/module4-local-debug.md (1)
91-100: 🩺 Stability & Availability | 🔵 TrivialWarn that zero-interval polling can overload the scheduler.
The guide recommends
--safe-interval 0and suggests reducing the minimum poll interval to0. Add an explicit warning that these settings are for isolated debugging only and must not be used against production/shared schedulers.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/source/tutorials/_notes/module4-local-debug.md` around lines 91 - 100, Add an explicit warning near the polling settings in module4-local-debug.md stating that zero-interval polling options, including --safe-interval 0 and set_minimum_job_poll_interval(0), can overload the scheduler and are only for isolated debugging, never production or shared schedulers.docs/source/tutorials/_notes/module7-plan.md (2)
101-107: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winResolve the live-cell requirement before implementation.
Line 105 limits Module 7 to one live cell, while line 106 asks to make as much as feasible executable. Choose the governing requirement and update the plan so build and test expectations are unambiguous.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/source/tutorials/_notes/module7-plan.md` around lines 101 - 107, Resolve the conflicting live-cell guidance in the Module 7 plan by selecting one governing requirement: either retain the single-live-cell limit or expand executable coverage where feasible. Update the “Tone and format” bullets to state the chosen scope clearly, including corresponding build and test expectations, while preserving the survey-module focus.
75-99: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winApply the accepted scope decisions to the proposed structure.
The notes reject the AiiDA Tutorials link and Slack invite, and request a shorter canonical-docs/blog pointer. The proposed plugin grid, domain-plugin card, archived tutorial link, and Slack reference still remain. Remove or replace these entries before authoring Module 7.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/source/tutorials/_notes/module7-plan.md` around lines 75 - 99, The Module 7 plan still includes rejected links and an overly detailed plugin section. In “Sec 5: The plugin ecosystem,” replace the domain-plugin card and broad grid details with a short link to the existing canonical plugin documentation plus the blog post on non-domain-specific plugins; in “Sec 6: Where to go next,” remove the AiiDA Tutorials and Slack references, and point readers to Discourse instead. Preserve the approved contributing guidance and ideas for future contributions.docs/source/tutorials/_notes/module5-plan.md (1)
17-18: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winMake Module 5’s data prerequisite deterministic.
The module depends on
tutorial/F-sweepand prior-module state, but the plan acknowledges that the group may be absent depending on build cache. A fresh downloaded notebook can therefore produce empty queries or fail. Bootstrap the required sweep when missing, or explicitly enforce and validate the module-order prerequisite.Also applies to: 80-88
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/source/tutorials/_notes/module5-plan.md` around lines 17 - 18, Update the Module 5 plan to make the tutorial/F-sweep and prior-module state prerequisite deterministic: either bootstrap the required sweep when it is absent, or explicitly enforce module order and validate that the expected populated provenance exists before running QueryBuilder. Ensure a fresh notebook cannot silently run against empty data or fail unexpectedly.docs/source/tutorials/_notes/module6-plan.md (1)
40-47: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReflect the settled error-handler scope.
Lines 72–77 decide that WorkGraph handlers belong in Module 6, but the checklist still says the decision is pending and the file list says the item may move. Update these statements to match the accepted scope before implementation.
Also applies to: 98-103
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/source/tutorials/_notes/module6-plan.md` around lines 40 - 47, The module6-plan notes still present WorkGraph error-handler coverage as undecided. Update the checklist and related decision/file-list statements around the error-handler entries to record that WorkGraph handlers belong in Module 6, removing any pending-decision or possible-move language while leaving Module 7’s broader error-handling scope intact.docs/source/tutorials/_notes/tutorial-proposals.md (1)
39-99: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMark this merged proposal as superseded.
This section still defines Module 7 as optional advanced topics, while
session-tasks.mdrecords a landed “Where to go next” module with a different scope. Add a dated historical/superseded label or update this proposal to avoid conflicting sources of truth.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/source/tutorials/_notes/tutorial-proposals.md` around lines 39 - 99, Mark the “Merged proposal” section in tutorial-proposals.md as superseded with a dated historical label, or revise it to match the landed “Where to go next” scope documented in session-tasks.md. Ensure it no longer presents Module 7 as the current optional advanced-topics definition or conflicts with the landed tutorial structure.docs/source/tutorials/_notes/driver.py (3)
13-13: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAssign exception message to a variable before raising.
As per coding guidelines: "Assign exception messages to a variable before raising, e.g.
msg = f'...'; raise TypeError(msg)."Proposed fix
- if V.ndim != 2: - raise ValueError('V must be a 2D array') + if V.ndim != 2: + msg = 'V must be a 2D array' + raise ValueError(msg)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/source/tutorials/_notes/driver.py` at line 13, In the validation branch of driver.py, assign the “V must be a 2D array” message to a local variable before raising ValueError, then raise using that variable.Source: Coding guidelines
11-20: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDocstrings missing or not Sphinx-style across all three new scripts. Per coding guidelines, docstrings should use
:param:/:return:/:raises:style with types kept in annotations;driver.pyandreaction-diffusion.pyhave no docstrings at all, andsemantic_linebreaks.pyhas plain one-line docstrings without:param:/:return:tags.
docs/source/tutorials/_notes/driver.py#L11-L20,L23-L156: add Sphinx-style docstrings tosave_pattern_pngandrun_reaction_diffusion_scan.docs/source/tutorials/_notes/reaction-diffusion.py#L14-L121: add Sphinx-style docstrings tofail,laplacian,simulate, andmain.docs/source/tutorials/_notes/semantic_linebreaks.py#L31-L98: extend existing one-line docstrings on_skip_line,_split_into_sentences,_reformat_paragraphwith:param:/:return:tags.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/source/tutorials/_notes/driver.py` around lines 11 - 20, Add Sphinx-style docstrings with :param:, :return:, and applicable :raises: entries to save_pattern_png and run_reaction_diffusion_scan in docs/source/tutorials/_notes/driver.py, and to fail, laplacian, simulate, and main in docs/source/tutorials/_notes/reaction-diffusion.py; document parameters and return types using existing annotations. In docs/source/tutorials/_notes/semantic_linebreaks.py, expand _skip_line, _split_into_sentences, and _reformat_paragraph from one-line docstrings to the same Sphinx format. Apply changes at the specified ranges in all three files.Source: Coding guidelines
66-79: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueNo timeout on subprocess call.
If
reaction-diffusion.pyhangs (e.g. numerical instability loop stalls), this call blocks indefinitely with notimeout=guard.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/source/tutorials/_notes/driver.py` around lines 66 - 79, Add a finite timeout to the subprocess.run invocation in the driver flow executing reaction-diffusion.py, preserving the existing arguments and output capture behavior. Handle the timeout according to the surrounding script’s established failure behavior so a hung subprocess does not block indefinitely.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@debug-module4.md`:
- Around line 35-61: Update the SSH setup instructions around the slurm_rsa copy
and slurm-ssh config entry to generate or obtain a per-run private key instead
of shipping the shared .github/config/slurm_rsa key. Replace disabled host
verification with a pinned container host key via UserKnownHostsFile and strict
checking, while preserving the existing slurm-ssh connection settings.
In `@docs/source/_ext/inline_downloads.py`:
- Around line 1-22: Prepend the repository’s standard copyright header, copied
exactly from an existing Python source file, before the module docstrings in
docs/source/_ext/inline_downloads.py,
docs/source/tutorials/include/setup_tutorial.py, and
docs/source/tutorials/include/setup_slurm.py; leave the remaining file contents
unchanged.
- Around line 61-94: Track whether each individual cell was modified separately
from the function-level return flag in _inline_run_cells. Reset a per-cell
modification flag before processing each cell, use it for the cell rewrite
condition, and update the existing modified flag only when a file is
successfully inlined so cells with missing targets retain their source and
execution state.
In `@docs/source/tutorials/_notes/asyncssh-issue-draft.md`:
- Around line 60-68: The draft should narrow its proposed behavior so resolved
empty known-hosts entries are treated as skip verification only for an explicit
opt-out combination such as StrictHostKeyChecking no with /dev/null. Preserve
verification behavior for ordinary missing or empty known_hosts files, and
update the “possible directions” wording to remove the broad
any-path/non-regular-file proposal.
In `@docs/source/tutorials/_notes/driver.py`:
- Around line 1-9: Add the repository-standard copyright header, copied from an
existing Python source file, at the top of
docs/source/tutorials/_notes/driver.py (lines 1-9),
docs/source/tutorials/_notes/reaction-diffusion.py (lines 1-8), and
docs/source/tutorials/_notes/semantic_linebreaks.py (lines 1-20). In
semantic_linebreaks.py, place the header before the module docstring; make no
other changes.
- Around line 66-79: Update the subprocess argument list in the driver’s
subprocess.run invocation to pass input_file as the positional input argument
expected by reaction-diffusion.py, removing the unsupported --input option.
Preserve the existing output option and invocation behavior.
In `@docs/source/tutorials/_notes/session-tasks.md`:
- Line 31: Update the release-readiness task around the M3 2D heatmap and its
RTD dependency: ensure the gsrd fix removing the TrivialStateError raise is
merged into aiidateam/gsrd:main, or configure the documentation build to pin or
override a known-good revision. Do not consider the docs cohort release-ready
until RTD uses a revision containing this fix.
In `@docs/source/tutorials/include/constants.py`:
- Around line 1-3: Add the project's standard AiiDA copyright header, copied
from an existing Python source file, at the beginning of
docs/source/tutorials/include/constants.py (lines 1-3),
docs/source/tutorials/include/plotting.py (lines 1-14),
docs/source/tutorials/include/tasks.py (lines 1-13), and
docs/source/tutorials/include/workflows.py (lines 1-9). Place it before each
module docstring and before plotting.py’s pyright suppression comment.
In `@docs/source/tutorials/include/setup_slurm.py`:
- Around line 46-47: Correct the repository root calculation in setup_slurm.py
by changing repo_root from parents[5] to parents[4], so slurm_key_src resolves
the repository’s .github/config/slurm_rsa path correctly.
In `@docs/source/tutorials/include/tasks.py`:
- Around line 122-129: Update the radial power averaging in the shown FFT
calculation to compute the radius-bin counts once, then divide by counts clamped
to at least one, matching plotting.py’s plot_fft_spectrum guard. Keep the
existing radial peak selection and wavelength calculation unchanged.
In `@docs/source/tutorials/module0.md`:
- Around line 51-58: Update the Setup note in module0.md to install gsrd from a
pinned released version or immutable commit instead of the mutable Git default
branch, matching the version or commit used by the shared setup script.
In `@docs/source/tutorials/module2.md`:
- Around line 390-398: Update the group population logic around sweep_group and
enriched_results so every newly generated calc_node is added to the existing or
newly created group, not only when created is true. Preserve the shared
'tutorial/F-sweep' group while moving node addition outside the created
conditional.
In `@docs/source/tutorials/module6.md`:
- Around line 32-38: Update the Module 6 setup cell after the tutorial profile
initialization to retrieve the persisted `gsrd_code` Code node and assign it to
the notebook variable before the first `pipeline_with_optional_fft.build(...)`
call, preserving compatibility with the shared tutorial profile.
In `@docs/source/tutorials/teaser.md`:
- Around line 17-174: Restore the teaser content in
docs/source/tutorials/teaser.md by removing the HTML comment wrappers so its
sections, examples, and module table render instead of only the title. Update
the “Error handlers, remote HPC, and more” table entry to link to the completed
Modules 4–7 using the repository’s established tutorial references.
In `@pyproject.toml`:
- Around line 300-304: Remove the tutorial VCS dependencies from the `tutorials`
optional-dependencies definition in `pyproject.toml` so they are not emitted
into published `Requires-Dist` metadata. Relocate the tutorial installation
configuration to an appropriate non-packaged setup, and remove the inline `gsrd`
TODO from the published configuration.
---
Minor comments:
In `@debug-module4.md`:
- Around line 165-166: Update the cleanup instructions near “Remove AiiDA
computer” to first remove the generated calculation and remote-data nodes, or
explicitly instruct users to discard the disposable tutorial profile, before
running “verdi computer delete slurm-ssh”. Ensure the sequence does not attempt
to delete a computer while linked provenance nodes remain.
- Around line 119-126: Update the launch example around input_path and
launch_shell_job so the input YAML path is resolved relative to a known
repository root, or explicitly change into that root before resolving it.
Preserve the existing input_path value and job-launch behavior while making the
guide independent of the caller’s current working directory.
In `@docs/source/tutorials/_notes/api-discrepancies.md`:
- Around line 169-177: Synchronize the module labels and discrepancy entries in
the table with the current tutorial plan defined in TRACKING.md, including
WorkGraph as Module 3, remote submission as Module 4, querying as Module 5, and
“Where to go next” as Module 7. Alternatively, explicitly label this table as a
historical snapshot if its existing numbering must remain unchanged.
In `@docs/source/tutorials/_notes/asyncssh-issue-draft.md`:
- Line 76: Update the reproduction environment note in the AsyncSSH issue draft
by replacing each <insert> placeholder with the exact AsyncSSH version, Python
version, and operating system used. Preserve the existing report format and
ensure no placeholders remain.
In `@docs/source/tutorials/_notes/module4-local-debug.md`:
- Around line 102-106: Update the tutorial’s AiiDA configuration example around
get_config().set_option to provide an explicit restoration path: capture the
original values before changing transport.task_retry_initial_interval and
transport.task_maximum_attempts, then restore those values after debugging, or
include exact commands using the documented defaults. Ensure subsequent AiiDA
operations are not affected by the temporary settings.
In `@docs/source/tutorials/_notes/module5-plan.md`:
- Around line 27-30: Update the “Export & share” tutorial step to materialize
QueryBuilder results into a group before invoking `verdi archive create`;
describe export using that group or explicit node identifiers, not a raw QB
selection, while preserving the existing light-touch scope.
In `@docs/source/tutorials/_notes/module7-plan.md`:
- Around line 63-73: Update the caching section’s enablement instructions so
caching is explicitly configured after selecting the tutorial profile, or
through a profile-local API. Ensure the ShellJob demonstration performs both
runs within that same tutorial profile and cannot reuse a cache entry from
another profile.
In `@docs/source/tutorials/_notes/session-tasks.md`:
- Around line 11-17: Update the historical “M7 expanded plan” entry in the
session-tasks tracker so its four questions are no longer presented as open;
mark them as resolved or otherwise clearly label the entry as historical, while
preserving the planned sections and context.
In `@docs/source/tutorials/include/setup_tutorial.py`:
- Around line 93-100: Update the broker startup wait around _broker.is_running
to detect when the 10-second deadline expires while the broker remains stopped,
and log a warning with clear startup-timeout context before continuing. Preserve
the existing polling behavior and avoid warning when the broker becomes running
within the deadline.
In `@docs/source/tutorials/module0.md`:
- Around line 213-228: Update the subprocess failure demonstration around
result.returncode to also print result.stdout alongside result.stderr,
preserving the existing exit-code output so readers can observe the missing “***
JOB DONE ***” marker described by the tutorial.
- Around line 114-133: Update the results.npz loading code around the np.load
call to use a context manager, ensuring the NpzFile handle closes after reading.
Retain the arrays needed by later cells, including params, rather than accessing
data after the context exits.
In `@docs/source/tutorials/module1.md`:
- Around line 292-305: Replace the shell-based tree command in the tutorial’s
dump-directory example with a Python pathlib listing using Path.rglob(), while
preserving the demonstration of the dumped directory contents and avoiding any
external tree utility dependency.
In `@docs/source/tutorials/module2.md`:
- Around line 371-373: Update the tutorial’s variables and surrounding prose to
consistently identify r['parsed']['variance_V'].creator as the parse_output
parser CalcFunctionNode, not a CalcJob or underlying ShellJob. Apply the same
terminology correction to the related content around lines 395-397; if
ShellJob-specific tagging or grouping is intended, explicitly traverse to that
node before applying it.
In `@docs/source/tutorials/module4.md`:
- Around line 287-290: Revise the tip in the module 4 tutorial to clarify that
AiiDA is not installed on the HPC, while the remote host still requires an SSH
server, SLURM, and the gsrd executable at /opt/gsrd/bin/gsrd. Remove the blanket
claim that nothing needs to be installed on the HPC, while preserving the
explanation that no sudo rights are required.
In `@docs/source/tutorials/module5.md`:
- Around line 302-304: Revise the “Python loop vs SQL” discussion to avoid
guaranteeing O(1) or O(log N) complexity for count(), SUM, and top-N queries.
State that these operations can be sublinear when suitable indexes and query
plans apply, while acknowledging complexity depends on factors such as
selectivity and index-only execution; retain the contrast that the example’s two
paths both visit every row.
In `@docs/source/tutorials/TRACKING.md`:
- Line 43: Resolve the unchecked Module 1 “Handling failures” item in
TRACKING.md by either completing and checking it off, or revising the Module 6
dependency wording to reflect the actual release state; apply the same
correction to the additional referenced checklist entries and keep the Module 7
completion status consistent.
---
Nitpick comments:
In @.github/workflows/docs-build.yml:
- Around line 68-110: The disabled module4 baking workflow block should not
remain as commented-out CI logic in .github/workflows/docs-build.yml. Remove the
entire commented block, including its trigger and commit steps, and track the
future implementation through a linked issue or concise TODO outside the
workflow instead.
In `@debug-module4.md`:
- Around line 85-87: Update the transport configuration section around the
core.ssh_async command to document the distinct core.ssh OpenSSH configuration
command and clearly state when to use it as the fallback. Ensure the documented
commands match the claimed asyncssh-first, OpenSSH-fallback behavior.
In `@docs/source/_ext/inline_downloads.py`:
- Around line 61-270: Add Sphinx-style :param: and :return: fields to the
docstrings of _inline_run_cells, _read_include, _convert_myst_block,
_inline_literalinclude, _convert_inline_roles, _clean_markdown, and
_process_markdown_cells in docs/source/_ext/inline_downloads.py (lines 61-270),
using existing annotations for types. Apply the same documentation update to
_container_reachable, _strip_legacy_block, and _strip_current_block in
docs/source/tutorials/include/setup_slurm.py (lines 65-93), adding :param: and
:return: fields without duplicating type information in the docstrings.
In `@docs/source/conf.py`:
- Around line 107-139: The disabled pre-executed-notebook design note in conf.py
should be moved to the project’s planning documentation, such as the existing
_notes/ or TRACKING.md, while preserving its intent and trade-offs. Remove the
multi-line commented block near nb_execution_excludepatterns and leave a concise
one-line pointer to the relocated note.
In `@docs/source/tutorials/_notes/driver.py`:
- Line 13: In the validation branch of driver.py, assign the “V must be a 2D
array” message to a local variable before raising ValueError, then raise using
that variable.
- Around line 11-20: Add Sphinx-style docstrings with :param:, :return:, and
applicable :raises: entries to save_pattern_png and run_reaction_diffusion_scan
in docs/source/tutorials/_notes/driver.py, and to fail, laplacian, simulate, and
main in docs/source/tutorials/_notes/reaction-diffusion.py; document parameters
and return types using existing annotations. In
docs/source/tutorials/_notes/semantic_linebreaks.py, expand _skip_line,
_split_into_sentences, and _reformat_paragraph from one-line docstrings to the
same Sphinx format. Apply changes at the specified ranges in all three files.
- Around line 66-79: Add a finite timeout to the subprocess.run invocation in
the driver flow executing reaction-diffusion.py, preserving the existing
arguments and output capture behavior. Handle the timeout according to the
surrounding script’s established failure behavior so a hung subprocess does not
block indefinitely.
In `@docs/source/tutorials/_notes/module4-local-debug.md`:
- Around line 91-100: Add an explicit warning near the polling settings in
module4-local-debug.md stating that zero-interval polling options, including
--safe-interval 0 and set_minimum_job_poll_interval(0), can overload the
scheduler and are only for isolated debugging, never production or shared
schedulers.
In `@docs/source/tutorials/_notes/module5-plan.md`:
- Around line 17-18: Update the Module 5 plan to make the tutorial/F-sweep and
prior-module state prerequisite deterministic: either bootstrap the required
sweep when it is absent, or explicitly enforce module order and validate that
the expected populated provenance exists before running QueryBuilder. Ensure a
fresh notebook cannot silently run against empty data or fail unexpectedly.
In `@docs/source/tutorials/_notes/module6-plan.md`:
- Around line 40-47: The module6-plan notes still present WorkGraph
error-handler coverage as undecided. Update the checklist and related
decision/file-list statements around the error-handler entries to record that
WorkGraph handlers belong in Module 6, removing any pending-decision or
possible-move language while leaving Module 7’s broader error-handling scope
intact.
In `@docs/source/tutorials/_notes/module7-plan.md`:
- Around line 101-107: Resolve the conflicting live-cell guidance in the Module
7 plan by selecting one governing requirement: either retain the
single-live-cell limit or expand executable coverage where feasible. Update the
“Tone and format” bullets to state the chosen scope clearly, including
corresponding build and test expectations, while preserving the survey-module
focus.
- Around line 75-99: The Module 7 plan still includes rejected links and an
overly detailed plugin section. In “Sec 5: The plugin ecosystem,” replace the
domain-plugin card and broad grid details with a short link to the existing
canonical plugin documentation plus the blog post on non-domain-specific
plugins; in “Sec 6: Where to go next,” remove the AiiDA Tutorials and Slack
references, and point readers to Discourse instead. Preserve the approved
contributing guidance and ideas for future contributions.
In `@docs/source/tutorials/_notes/start-slurm-container.sh`:
- Around line 42-52: In the uv setup block, validate that the `uv` executable
lookup succeeds before calling `readlink -f` or `docker cp`. Update the
`UV_HOST` initialization around `which uv` to fail clearly with an actionable
message when uv is absent, while preserving the existing copy and container
installation flow when it is found.
In `@docs/source/tutorials/_notes/tutorial-proposals.md`:
- Around line 39-99: Mark the “Merged proposal” section in tutorial-proposals.md
as superseded with a dated historical label, or revise it to match the landed
“Where to go next” scope documented in session-tasks.md. Ensure it no longer
presents Module 7 as the current optional advanced-topics definition or
conflicts with the landed tutorial structure.
In `@docs/source/tutorials/include/plotting.py`:
- Around line 34-46: Update the docstring of plot_provenance to document node
with a Sphinx-style :param: entry and the returned Graphviz Digraph with a
:return: entry, while keeping types in the existing annotations and preserving
the current behavior.
In `@docs/source/tutorials/include/tasks.py`:
- Around line 106-129: Extract the shared V_final loading, centring, 2D FFT, and
radial power-spectrum calculation from fft_peak_wavelength in
docs/source/tutorials/include/tasks.py:106-129 into a common helper, including
the existing zero-radius guard. Update fft_peak_wavelength to call that helper,
and update plot_fft_spectrum in
docs/source/tutorials/include/plotting.py:154-206 to reuse it instead of
duplicating the FFT and radial-averaging logic; both sites require changes.
- Around line 32-36: Update the docstrings for prepare_input and bump_n_steps to
include Sphinx-style :param: documentation for each parameter, matching the
existing documentation style and relying on type annotations for parameter
types. Preserve their current summaries and behavior.
In `@docs/source/tutorials/module3.md`:
- Around line 223-231: Update the tutorial examples around the WorkGraph
input/output listings to use public aiida-workgraph accessors instead of private
`_get_keys()` and `._value`; if those accessors are unavailable, isolate the
private calls in a compatibility helper, and pin/document the supported
aiida-workgraph version. Apply the same change to the corresponding sections
referenced by the comment.
In `@docs/source/tutorials/module6.md`:
- Around line 601-604: Update the tutorial’s adaptive sweep output access in
plot_adaptive_sweep and the additionally affected cells to use AiiDA WorkGraph’s
supported public accessor for dynamic namespace outputs instead of the private
_value attribute, preserving the existing coarse_variances and refined_variances
values passed to the plotting function.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| The container's SSH user is `xenon`, password `javagat`. The pre-generated | ||
| key is in `.github/config/slurm_rsa`. | ||
|
|
||
| ```fish | ||
| set REPO_ROOT (git rev-parse --show-toplevel) | ||
| cp $REPO_ROOT/.github/config/slurm_rsa ~/.ssh/slurm_rsa | ||
| chmod 600 ~/.ssh/slurm_rsa | ||
| ``` | ||
|
|
||
| Check if the SSH config entry already exists: | ||
|
|
||
| ```fish | ||
| grep -c "AiiDA tutorial" ~/.ssh/config | ||
| ``` | ||
|
|
||
| If it prints `0`, add it: | ||
|
|
||
| ```fish | ||
| printf '\n# --- AiiDA tutorial (slurm-ssh) --- | ||
| Host slurm-ssh | ||
| HostName localhost | ||
| User xenon | ||
| Port 5001 | ||
| IdentityFile ~/.ssh/slurm_rsa | ||
| StrictHostKeyChecking no | ||
| UserKnownHostsFile /dev/null | ||
| LogLevel ERROR\n' >> ~/.ssh/config |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
git ls-files --error-unmatch .github/config/slurm_rsa
head -n 1 .github/config/slurm_rsaRepository: aiidateam/aiida-core
Length of output: 1992
Don't ship a shared SSH key or disable host verification here. slurm_rsa is copied into ~/.ssh, and StrictHostKeyChecking no plus UserKnownHostsFile /dev/null removes host authentication. Use a per-run key and pin the container host key instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@debug-module4.md` around lines 35 - 61, Update the SSH setup instructions
around the slurm_rsa copy and slurm-ssh config entry to generate or obtain a
per-run private key instead of shipping the shared .github/config/slurm_rsa key.
Replace disabled host verification with a pinned container host key via
UserKnownHostsFile and strict checking, while preserving the existing slurm-ssh
connection settings.
| """Sphinx extension: make downloaded notebooks self-contained and Jupyter-friendly. | ||
|
|
||
| After the build, post-process every ``.ipynb`` in ``_downloads/``: | ||
|
|
||
| 1. Replace ``%run -i <path>`` code cells with inlined file contents. | ||
| 2. Convert MyST admonitions to HTML ``<div class="alert ...">`` blocks. | ||
| 3. Convert MyST dropdowns to ``<details>`` elements (with literalinclude inlined). | ||
| 4. Strip MyST-only inline roles to plain text. | ||
| 5. Remove target labels and self-referential download links. | ||
| """ | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import json | ||
| import re | ||
| from pathlib import Path | ||
| from typing import Any | ||
|
|
||
| from sphinx.application import Sphinx | ||
| from sphinx.util import logging | ||
|
|
||
| logger = logging.getLogger(__name__) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
New Python files are missing the standard copyright header. Coding guidelines require every new source file to include the standard copyright header copied from an existing .py file; none of the three new files in this cohort have one.
docs/source/_ext/inline_downloads.py#L1-L22: prepend the standard header before the module docstring.docs/source/tutorials/include/setup_tutorial.py#L1-L26: prepend the standard header before the module docstring.docs/source/tutorials/include/setup_slurm.py#L1-L44: prepend the standard header before the module docstring.
As per coding guidelines, "New source files must include the standard copyright header copied from an existing .py file."
📍 Affects 3 files
docs/source/_ext/inline_downloads.py#L1-L22(this comment)docs/source/tutorials/include/setup_tutorial.py#L1-L26docs/source/tutorials/include/setup_slurm.py#L1-L44
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/source/_ext/inline_downloads.py` around lines 1 - 22, Prepend the
repository’s standard copyright header, copied exactly from an existing Python
source file, before the module docstrings in
docs/source/_ext/inline_downloads.py,
docs/source/tutorials/include/setup_tutorial.py, and
docs/source/tutorials/include/setup_slurm.py; leave the remaining file contents
unchanged.
Source: Coding guidelines
| def _inline_run_cells(cells: list[dict], source_dir: Path) -> bool: | ||
| """Replace ``%run -i <path>`` cells with inlined file contents.""" | ||
| modified = False | ||
|
|
||
| for cell in cells: | ||
| if cell.get('cell_type') != 'code': | ||
| continue | ||
|
|
||
| source = ''.join(cell.get('source', [])) | ||
| matches = list(_RUN_PATTERN.finditer(source)) | ||
| if not matches: | ||
| continue | ||
|
|
||
| new_parts: list[str] = [] | ||
| for match in matches: | ||
| rel_path = match.group(1).strip().strip('\'"') | ||
| include_file = source_dir / rel_path | ||
| if not include_file.is_file(): | ||
| logger.warning('inline_downloads: file not found: %s', include_file) | ||
| new_parts.append(match.group(0)) | ||
| continue | ||
| content = include_file.read_text(encoding='utf-8') | ||
| new_parts.append(f'# — inlined from {rel_path} —\n{content}') | ||
| modified = True | ||
|
|
||
| if modified: | ||
| remaining = _RUN_PATTERN.sub('', source).strip() | ||
| if remaining: | ||
| new_parts.append(remaining) | ||
| cell['source'] = ['\n\n'.join(new_parts)] | ||
| cell['outputs'] = [] | ||
| cell['execution_count'] = None | ||
|
|
||
| return modified |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
modified flag leaks across cells, causing unrelated cells to have outputs wiped.
modified is set once at function scope and never reset per cell. If an earlier code cell successfully inlines a %run -i file, modified stays True for the rest of the loop. A later cell that also matches _RUN_PATTERN but whose target file is missing will then still satisfy if modified: and get its source/outputs/execution_count rewritten — even though nothing was inlined for that cell.
🐛 Proposed fix: track modification per cell
for cell in cells:
if cell.get('cell_type') != 'code':
continue
source = ''.join(cell.get('source', []))
matches = list(_RUN_PATTERN.finditer(source))
if not matches:
continue
new_parts: list[str] = []
+ cell_modified = False
for match in matches:
rel_path = match.group(1).strip().strip('\'"')
include_file = source_dir / rel_path
if not include_file.is_file():
logger.warning('inline_downloads: file not found: %s', include_file)
new_parts.append(match.group(0))
continue
content = include_file.read_text(encoding='utf-8')
new_parts.append(f'# — inlined from {rel_path} —\n{content}')
- modified = True
+ cell_modified = True
- if modified:
+ if cell_modified:
remaining = _RUN_PATTERN.sub('', source).strip()
if remaining:
new_parts.append(remaining)
cell['source'] = ['\n\n'.join(new_parts)]
cell['outputs'] = []
cell['execution_count'] = None
+ modified = True📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def _inline_run_cells(cells: list[dict], source_dir: Path) -> bool: | |
| """Replace ``%run -i <path>`` cells with inlined file contents.""" | |
| modified = False | |
| for cell in cells: | |
| if cell.get('cell_type') != 'code': | |
| continue | |
| source = ''.join(cell.get('source', [])) | |
| matches = list(_RUN_PATTERN.finditer(source)) | |
| if not matches: | |
| continue | |
| new_parts: list[str] = [] | |
| for match in matches: | |
| rel_path = match.group(1).strip().strip('\'"') | |
| include_file = source_dir / rel_path | |
| if not include_file.is_file(): | |
| logger.warning('inline_downloads: file not found: %s', include_file) | |
| new_parts.append(match.group(0)) | |
| continue | |
| content = include_file.read_text(encoding='utf-8') | |
| new_parts.append(f'# — inlined from {rel_path} —\n{content}') | |
| modified = True | |
| if modified: | |
| remaining = _RUN_PATTERN.sub('', source).strip() | |
| if remaining: | |
| new_parts.append(remaining) | |
| cell['source'] = ['\n\n'.join(new_parts)] | |
| cell['outputs'] = [] | |
| cell['execution_count'] = None | |
| return modified | |
| def _inline_run_cells(cells: list[dict], source_dir: Path) -> bool: | |
| """Replace ``%run -i <path>`` cells with inlined file contents.""" | |
| modified = False | |
| for cell in cells: | |
| if cell.get('cell_type') != 'code': | |
| continue | |
| source = ''.join(cell.get('source', [])) | |
| matches = list(_RUN_PATTERN.finditer(source)) | |
| if not matches: | |
| continue | |
| new_parts: list[str] = [] | |
| cell_modified = False | |
| for match in matches: | |
| rel_path = match.group(1).strip().strip('\'"') | |
| include_file = source_dir / rel_path | |
| if not include_file.is_file(): | |
| logger.warning('inline_downloads: file not found: %s', include_file) | |
| new_parts.append(match.group(0)) | |
| continue | |
| content = include_file.read_text(encoding='utf-8') | |
| new_parts.append(f'# — inlined from {rel_path} —\n{content}') | |
| cell_modified = True | |
| if cell_modified: | |
| remaining = _RUN_PATTERN.sub('', source).strip() | |
| if remaining: | |
| new_parts.append(remaining) | |
| cell['source'] = ['\n\n'.join(new_parts)] | |
| cell['outputs'] = [] | |
| cell['execution_count'] = None | |
| modified = True | |
| return modified |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/source/_ext/inline_downloads.py` around lines 61 - 94, Track whether
each individual cell was modified separately from the function-level return flag
in _inline_run_cells. Reset a per-cell modification flag before processing each
cell, use it for the cell rewrite condition, and update the existing modified
flag only when a file is successfully inlined so cells with missing targets
retain their source and execution state.
| ## What I think the expected behavior is | ||
|
|
||
| When `~/.ssh/config` specifies `UserKnownHostsFile /dev/null` (or any path that resolves to an empty/non-existent file) *combined with* `StrictHostKeyChecking no`, OpenSSH's semantics are "skip verification entirely." Mapping this onto asyncssh's existing semantics, this is equivalent to `known_hosts=None`. | ||
|
|
||
| A few possible directions, in roughly increasing scope: | ||
|
|
||
| 1. **Treat config-resolved `[]` + `StrictHostKeyChecking no` as `known_hosts=None`.** Smallest, most targeted: only when the user explicitly opted out of strict checking does the empty-list case become "skip" rather than "refuse." Honors the intent of the combined directives. | ||
| 2. **Recognize `/dev/null` (or any non-regular file) in `UserKnownHostsFile` as "skip verification."** OS-level semantics: writing to `/dev/null` is the universal "discard" sentinel; resolving it to `[]` then enforcing strict matching is the surprising step. | ||
| 3. **Document the asymmetry explicitly.** If the current behavior is intentional, a doc note that "an empty list resolved from ssh_config triggers strict refusal, while an explicit empty list passed as kwarg skips verification" would at least make this predictable for callers. |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Narrow the proposed “skip verification” behavior.
The draft’s broad wording around any empty/non-existent or non-regular known-hosts path conflicts with Line 74: the same resolved [] value can represent a first-time user with no known_hosts file. Treating both cases as known_hosts=None could silently disable host-key verification. Restrict the proposal to an explicit opt-out such as StrictHostKeyChecking no with /dev/null, while preserving verification for ordinary missing files.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/source/tutorials/_notes/asyncssh-issue-draft.md` around lines 60 - 68,
The draft should narrow its proposed behavior so resolved empty known-hosts
entries are treated as skip verification only for an explicit opt-out
combination such as StrictHostKeyChecking no with /dev/null. Preserve
verification behavior for ordinary missing or empty known_hosts files, and
update the “possible directions” wording to remove the broad
any-path/non-regular-file proposal.
| import subprocess | ||
| import sys | ||
| import tempfile | ||
| from pathlib import Path | ||
|
|
||
| import matplotlib.pyplot as plt | ||
| import numpy as np | ||
| import yaml | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Missing standard copyright header on all three new dev scripts. Each new .py file omits the required copyright header, per repo coding guidelines. As per coding guidelines: "New source files must include the standard copyright header copied from an existing .py file."
docs/source/tutorials/_notes/driver.py#L1-L9: add the standard header at the top of the file.docs/source/tutorials/_notes/reaction-diffusion.py#L1-L8: add the standard header at the top of the file.docs/source/tutorials/_notes/semantic_linebreaks.py#L1-L20: add the standard header above the module docstring.
📍 Affects 3 files
docs/source/tutorials/_notes/driver.py#L1-L9(this comment)docs/source/tutorials/_notes/reaction-diffusion.py#L1-L8docs/source/tutorials/_notes/semantic_linebreaks.py#L1-L20
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/source/tutorials/_notes/driver.py` around lines 1 - 9, Add the
repository-standard copyright header, copied from an existing Python source
file, at the top of docs/source/tutorials/_notes/driver.py (lines 1-9),
docs/source/tutorials/_notes/reaction-diffusion.py (lines 1-8), and
docs/source/tutorials/_notes/semantic_linebreaks.py (lines 1-20). In
semantic_linebreaks.py, place the header before the module docstring; make no
other changes.
Source: Coding guidelines
| :::{note} Setup | ||
| This first module uses only the `gsrd` simulator, no AiiDA yet. | ||
| Install it with: | ||
|
|
||
| ```bash | ||
| pip install git+https://github.com/aiidateam/gsrd | ||
| ``` | ||
| ::: |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Pin the gsrd dependency.
The tutorial installs from the mutable Git default branch, while its output filenames, diagnostics, and images depend on exact CLI behavior. Use a released version or immutable commit, consistently with the shared setup script.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/source/tutorials/module0.md` around lines 51 - 58, Update the Setup note
in module0.md to install gsrd from a pinned released version or immutable commit
instead of the mutable Git default branch, matching the version or commit used
by the shared setup script.
| sweep_group: orm.Group | ||
| created: bool | ||
| sweep_group, created = orm.Group.collection.get_or_create('tutorial/F-sweep') | ||
|
|
||
| if created: | ||
| for r in enriched_results: | ||
| calc_node = r['parsed']['variance_V'].creator | ||
| sweep_group.add_nodes(calc_node) | ||
|
|
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Keep group membership correct on reruns.
The shared tutorial profile persists across executions. When the group already exists, if created skips adding the newly generated nodes, leaving the group stale while extras and queries include the new runs. Add the current nodes unconditionally or use a per-run group.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/source/tutorials/module2.md` around lines 390 - 398, Update the group
population logic around sweep_group and enriched_results so every newly
generated calc_node is added to the existing or newly created group, not only
when created is true. Preserve the shared 'tutorial/F-sweep' group while moving
node addition outside the created conditional.
| ```{code-cell} ipython3 | ||
| # Set up the tutorial's isolated sandbox profile (same as Module 1). | ||
| # `%load_ext aiida` enables the `%verdi` magic; `%run` creates or loads the | ||
| # shared `tutorial-<hash>` profile, so data from earlier modules is available. | ||
| %load_ext aiida | ||
| %run -i include/setup_tutorial.py | ||
| ``` |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Load gsrd_code in this notebook.
The setup cell only loads the AiiDA profile; it does not recreate the Python variable gsrd_code. A fresh downloaded notebook therefore reaches the first pipeline_with_optional_fft.build(...) call with gsrd_code undefined, despite the persisted code node existing in the profile.
Proposed fix
%load_ext aiida
%run -i include/setup_tutorial.py
+
+from aiida.orm import load_code
+
+gsrd_code = load_code('gsrd@localhost')📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ```{code-cell} ipython3 | |
| # Set up the tutorial's isolated sandbox profile (same as Module 1). | |
| # `%load_ext aiida` enables the `%verdi` magic; `%run` creates or loads the | |
| # shared `tutorial-<hash>` profile, so data from earlier modules is available. | |
| %load_ext aiida | |
| %run -i include/setup_tutorial.py | |
| ``` |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/source/tutorials/module6.md` around lines 32 - 38, Update the Module 6
setup cell after the tutorial profile initialization to retrieve the persisted
`gsrd_code` Code node and assign it to the notebook variable before the first
`pipeline_with_optional_fft.build(...)` call, preserving compatibility with the
shared tutorial profile.
| <!-- This page gives you a quick preview of what you will be able to do after completing the tutorial. --> | ||
| <!-- Don't worry about understanding every detail -- the modules will teach you each piece step by step. --> | ||
| <!----> | ||
| <!-- ## The scenario --> | ||
| <!----> | ||
| <!-- Imagine you want to scan 20 values of an input parameter, collect the results, and analyze how an output quantity varies across the scan. --> | ||
| <!----> | ||
| <!-- Without AiiDA, this typically means a script full of `subprocess` calls, temporary directories, manual file bookkeeping, and no record of what ran or why. --> | ||
| <!-- If a few runs fail, you notice only when you parse the results -- if you notice at all. --> | ||
| <!----> | ||
| <!-- With AiiDA, you get **monitoring**, **automatic error recovery**, **queryable results**, and **full provenance** -- for free. --> | ||
| <!----> | ||
| <!-- ## Monitoring running processes --> | ||
| <!----> | ||
| <!-- Once you submit a parameter sweep, you can check its status at any time from the command line: --> | ||
| <!----> | ||
| <!-- ```console --> | ||
| <!-- $ verdi process list --> | ||
| <!-- PK Created Process label State Process status --> | ||
| <!-- ---- --------- -------------------- ---------------- -------------------------- --> | ||
| <!-- 142 2m ago GrayScottSweep ⏵ Waiting Waiting for 20 sub-processes --> | ||
| <!-- 143 2m ago run_simulation ✓ Finished [0] --> | ||
| <!-- 144 2m ago run_simulation ✓ Finished [0] --> | ||
| <!-- 145 1m ago run_simulation ⏵ Running --> | ||
| <!-- ... --> | ||
| <!----> | ||
| <!-- Total results: 22 --> | ||
| <!-- ``` --> | ||
| <!----> | ||
| <!-- You can drill into any process to see its inputs, outputs, and log messages: --> | ||
| <!----> | ||
| <!-- ```console --> | ||
| <!-- $ verdi process show 145 --> | ||
| <!-- Process label: run_simulation --> | ||
| <!-- Process state: Running --> | ||
| <!-- Exit code: None --> | ||
| <!----> | ||
| <!-- Inputs: --> | ||
| <!-- F 0.042 --> | ||
| <!-- k 0.065 --> | ||
| <!-- n_steps 5000 --> | ||
| <!-- ... --> | ||
| <!-- ``` --> | ||
| <!----> | ||
| <!-- Even if you close your terminal and come back the next day, all processes and their results are safely stored in the database. --> | ||
| <!----> | ||
| <!-- ## Automatic error recovery --> | ||
| <!----> | ||
| <!-- Some parameter combinations cause the calculation to fail. --> | ||
| <!-- A plain script would just skip these and move on, losing information. --> | ||
| <!----> | ||
| <!-- With AiiDA workflows, you can register **error handlers** that automatically respond to specific exit codes. --> | ||
| <!-- For instance, when a calculation fails, the workflow can adjust inputs and retry: --> | ||
| <!----> | ||
| <!-- ```console --> | ||
| <!-- $ verdi process status 142 --> | ||
| <!-- GrayScottSweep<142> [Waiting] --> | ||
| <!-- ├── run_simulation<143> [Finished] [0] --> | ||
| <!-- ├── run_simulation<144> [Finished] [0] --> | ||
| <!-- ├── run_simulation<145> [Finished] [30]: Trivial steady state --> | ||
| <!-- │ └── [handler: retry_with_more_steps] → retrying with n_steps=10000 --> | ||
| <!-- ├── run_simulation<146> [Finished] [0] ← retry succeeded --> | ||
| <!-- ... --> | ||
| <!-- ``` --> | ||
| <!----> | ||
| <!-- The failure, the handler decision, and the retry are all recorded in the provenance -- nothing is lost or hidden. --> | ||
| <!----> | ||
| <!-- ## Querying your results --> | ||
| <!----> | ||
| <!-- After the sweep finishes, you don't need to remember file paths or dig through directories. --> | ||
| <!-- AiiDA stores everything in a database that you can query with Python: --> | ||
| <!----> | ||
| <!-- ```python --> | ||
| <!-- from aiida import orm --> | ||
| <!----> | ||
| <!-- qb = orm.QueryBuilder() --> | ||
| <!-- qb.append(orm.CalcFunctionNode, filters={'label': 'run_simulation'}) --> | ||
| <!-- qb.append(orm.Float, filters={'label': 'variance_V'}, with_incoming='*') --> | ||
| <!----> | ||
| <!-- for node, in qb.iterall(): --> | ||
| <!-- calc = node.creator --> | ||
| <!-- F = calc.inputs.parameters.get_dict()['F'] --> | ||
| <!-- print(f"F = {F:.3f} → variance(V) = {node.value:.4e}") --> | ||
| <!-- ``` --> | ||
| <!----> | ||
| <!-- ```text --> | ||
| <!-- F = 0.035 → variance(V) = 1.2345e-03 --> | ||
| <!-- F = 0.037 → variance(V) = 2.8901e-03 --> | ||
| <!-- F = 0.039 → variance(V) = 5.1234e-03 --> | ||
| <!-- ... --> | ||
| <!-- ``` --> | ||
| <!----> | ||
| <!-- This works whether the data was produced five minutes ago or five months ago. --> | ||
| <!-- You can filter, sort, and join across any combination of inputs, outputs, and process metadata. --> | ||
| <!----> | ||
| <!-- ## Full provenance --> | ||
| <!----> | ||
| <!-- Every piece of data in AiiDA is connected to a **provenance graph** that records exactly how it was produced: --> | ||
| <!----> | ||
| <!-- ```console --> | ||
| <!-- $ verdi node graph generate 146 --> | ||
| <!-- ``` --> | ||
| <!----> | ||
| <!-- <!-- TODO: Add a pre-generated provenance graph image here showing: --> | ||
| <!-- parameters → run_simulation → variance_V / mean_V --> | ||
| <!-- with the retry chain visible --> --> | ||
| <!----> | ||
| <!-- The graph answers questions that are otherwise impossible to reconstruct: --> | ||
| <!-- - *"What parameters produced this outlier?"* --> | ||
| <!-- - *"Did this result come from the original run or a retry?"* --> | ||
| <!-- - *"Which version of the simulation script was used?"* --> | ||
| <!----> | ||
| <!-- ## Sharing and reproducing results --> | ||
| <!----> | ||
| <!-- You can export any set of nodes -- including their full provenance -- to an archive file that a colleague can import into their own AiiDA database: --> | ||
| <!----> | ||
| <!-- ```console --> | ||
| <!-- $ verdi archive create sweep_results.aiida --all --> | ||
| <!-- $ verdi archive import sweep_results.aiida # on another machine --> | ||
| <!-- ``` --> | ||
| <!----> | ||
| <!-- The imported data carries its complete history: every input, every intermediate step, every output. --> | ||
| <!-- Reproducibility is not an afterthought -- it is built into the data model. --> | ||
| <!----> | ||
| <!-- ## Extending a workflow --> | ||
| <!----> | ||
| <!-- Suppose you later realize you need an additional analysis step -- for example, a post-processing calculation that extracts a derived quantity from the raw output. --> | ||
| <!-- With WorkGraph, extending the workflow is just adding a task and connecting it: --> | ||
| <!----> | ||
| <!-- ```python --> | ||
| <!-- from aiida_workgraph import task --> | ||
| <!----> | ||
| <!-- @task --> | ||
| <!-- def post_process(raw_output): --> | ||
| <!-- """Derive a new quantity from the calculation output.""" --> | ||
| <!-- # ... your analysis code ... --> | ||
| <!-- return derived_quantity --> | ||
| <!----> | ||
| <!-- # Add the new step to an existing workflow — one line: --> | ||
| <!-- wg.add_task(post_process, raw_output=wg.tasks.calculate.outputs.result) --> | ||
| <!-- ``` --> | ||
| <!----> | ||
| <!-- No class hierarchies to subclass, no `define()` to override, no outline to restructure. --> | ||
| <!-- You compose tasks like functions -- and AiiDA tracks the provenance of the new step automatically. --> | ||
| <!----> | ||
| <!-- ## What this tutorial will teach you --> | ||
| <!----> | ||
| <!-- The modules below will take you from zero to everything shown on this page: --> | ||
| <!----> | ||
| <!-- | What you saw above | Where you'll learn it | --> | ||
| <!-- |---|---| --> | ||
| <!-- | The simulation code and its interface | {ref}`Module 0 <tutorial:module0>` | --> | ||
| <!-- | Running and tracking simulations | {ref}`Module 1 <tutorial:module1>` | --> | ||
| <!-- | Richer data types, calcfunctions, parameter sweeps | {ref}`Module 2 <tutorial:module2>` | --> | ||
| <!-- | Building workflows | {ref}`Module 3 <tutorial:module3>` | --> | ||
| <!-- | Error handlers, remote HPC, and more | Coming soon | --> | ||
| <!----> | ||
| <!-- Ready? Start with {ref}`Module 0 <tutorial:module0>` to meet the running example, then head to {ref}`Module 1 <tutorial:module1>`. --> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Do not ship the teaser as an entirely commented-out page.
The rendered page contains only the title; all useful content is hidden. Once restored, update the stale “Coming soon” entry to link to the completed Modules 4–7.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/source/tutorials/teaser.md` around lines 17 - 174, Restore the teaser
content in docs/source/tutorials/teaser.md by removing the HTML comment wrappers
so its sections, examples, and module table render instead of only the title.
Update the “Error handlers, remote HPC, and more” table entry to link to the
completed Modules 4–7 using the repository’s established tutorial references.
| tutorials = [ | ||
| 'aiida-shell @ git+https://github.com/aiidateam/aiida-shell.git@master', | ||
| 'aiida-workgraph @ git+https://github.com/GeigerJ2/aiida-workgraph.git@fix/map-zone-output-retrieval', | ||
| 'gsrd @ git+https://github.com/GeigerJ2/gsrd.git@fix/dont-raise-on-trivial-state' # TODO: revert to aiidateam/gsrd@main once the PR merges | ||
| ] |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## pyproject slice ##\n'
nl -ba pyproject.toml | sed -n '280,320p'
printf '\n## tutorial references ##\n'
rg -n "tutorials|optional-dependencies|aiida-shell|aiida-workgraph|gsrd" pyproject.toml README* docs .github || trueRepository: aiidateam/aiida-core
Length of output: 1963
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
print("Need to inspect package-upload rules for direct URL deps and optional dependencies.")
PYRepository: aiidateam/aiida-core
Length of output: 1963
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "noop"Repository: aiidateam/aiida-core
Length of output: 1963
🌐 Web query:
PyPI Warehouse direct dependency URL references Requires-Dist optional-dependencies PEP 621 upload rejection documentation
💡 Result:
PyPI (via the Warehouse project) explicitly rejects any package upload that includes direct dependency URL references in its metadata [1][2][3]. This restriction applies to the Requires-Dist field within a distribution's metadata, regardless of whether those dependencies are defined in a project's standard dependencies or its optional-dependencies (extras) [2][3]. The reasoning behind this policy is rooted in security and the maintenance of a reliable, predictable package index [4]. While PEP 508 permits the syntax for direct URL references (e.g., pkg @ git+https://... or pkg @ https://...) for use by developers, individual projects, or private index servers [4][5][6], PEP 440 explicitly states that public index servers like PyPI should not allow these references in uploaded distributions [1][2]. By prohibiting direct URLs in Requires-Dist: 1. PyPI avoids becoming a vector for arbitrary code execution or supply chain attacks that might result from dependencies hosted on unverified or untrusted external servers [4]. 2. It ensures that all dependencies listed in a package on PyPI can be reliably resolved and installed from PyPI (or other configured trusted indices), rather than relying on the availability or integrity of third-party URLs [1][7]. This restriction holds even when using modern metadata standards like PEP 621 (which allows storing core metadata in pyproject.toml) [8][9]. While PEP 621 standardizes how dependencies are defined, it does not override the fundamental prohibition against direct references on public indices enforced by the underlying packaging infrastructure [1][6]. Attempting to upload a package containing these references will trigger a 400 Bad Request error from the PyPI API, typically indicating that direct dependencies are not permitted [1][2][7]. Developers needing to reference non-PyPI code are encouraged to use alternative installation workflows, such as requirements files or environment markers, rather than embedding the URL directly in the package metadata [3][6].
Citations:
- 1: Allow PEP 508 URL requirements in requires_dist pypi/warehouse#7136
- 2: twine check fails to prevent Invalid value for requires_dist. Error: Can't have direct dependency pypa/twine#726
- 3: [BUG] Avoid depdencies with URLs in setup.cfg pypa/setuptools#2880
- 4: Allow direct urls in install_requires pypa/pip#6301
- 5: https://peps.python.org/pep-0508/
- 6: https://setuptools.pypa.io/en/stable/userguide/dependency_management.html
- 7: Cannot upload with external dependency due to "Invalid value for requires_dist" pypi/warehouse#9404
- 8: https://peps.python.org/pep-0621/
- 9: https://pdm-project.org/en/stable/reference/pep621/
🌐 Web query:
PEP 621 optional-dependencies direct URL references Requires-Dist PyPI Warehouse rejects direct dependencies
💡 Result:
PyPI (via the Warehouse project) does not permit the upload of distributions that contain direct URL references (also known as direct references) in their Requires-Dist metadata [1][2][3]. This restriction applies to all dependency fields, including project.dependencies and project.optional-dependencies defined under PEP 621 [4][1]. If you attempt to upload a package to PyPI that includes a direct URL dependency (e.g., a VCS link or a direct wheel URL) in its metadata, the PyPI server will reject the upload and return an HTTP 400 Client Error, typically stating: "Invalid value for requires_dist. Error: Can't have direct dependency" [2][3][5]. This policy is maintained for several reasons: 1. Consistency with Packaging Standards: While PEP 508 defines the format for direct references, it explicitly notes that public index servers are not required to support them and typically should not allow them in uploaded distributions to ensure package integrity and index reliability [6][2]. 2. Security and Reliability: Restricting direct URLs acts as a security layer, preventing packages hosted on PyPI from silently triggering downloads from untrusted, arbitrary external sources [7]. 3. Index Integrity: PyPI is intended to be a reliable, centralized index. Allowing direct URL dependencies would undermine this by introducing dependencies that the index itself cannot verify, host, or guarantee the availability of [1][8]. While tools like pip can successfully install packages containing direct URL dependencies (when installing from a local path, a requirements file, or a specific URL), publishing these packages to a public index like PyPI is not supported [1][9]. If your project requires a dependency not available on PyPI, recommended best practices include: - Creating a separate requirements file (e.g., requirements.txt) that includes the direct URL, which users can install alongside your package [8]. - Documenting the requirement clearly for your users [8]. - Uploading the missing dependency to PyPI if possible, or using a private index server that supports your specific configuration requirements [3][7].
Citations:
- 1: https://setuptools.pypa.io/en/latest/userguide/dependency%5Fmanagement.html
- 2: Allow PEP 508 URL requirements in requires_dist pypi/warehouse#7136
- 3: Cannot upload with external dependency due to "Invalid value for requires_dist" pypi/warehouse#9404
- 4: https://peps.python.org/pep-0621/
- 5: Cannot upload Python wheels with direct references in the Requires-Dist field pypa/twine#486
- 6: https://peps.python.org/pep-0508/
- 7: Allow direct urls in install_requires pypa/pip#6301
- 8: https://stackoverflow.com/questions/77134254/specify-dependencies-in-pyproject-toml-with-install-url-or-with-index-url
- 9: https://setuptools.pypa.io/en/stable/userguide/dependency_management.html
Keep tutorial VCS dependencies out of published metadata pyproject.toml:300-304
PyPI rejects uploads when Requires-Dist contains direct URL/VCS references, and extras in [project.optional-dependencies] still end up in that metadata. Moving these tutorial-only installs out of the packaged pyproject.toml keeps the next release upload from failing. The gsrd entry’s TODO should be tracked separately.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pyproject.toml` around lines 300 - 304, Remove the tutorial VCS dependencies
from the `tutorials` optional-dependencies definition in `pyproject.toml` so
they are not emitted into published `Requires-Dist` metadata. Relocate the
tutorial installation configuration to an appropriate non-packaged setup, and
remove the inline `gsrd` TODO from the published configuration.
Rename the first card on the documentation landing page from "Introduction" to "About", and its button from "To the introduction" to "To the overview". Group-meeting feedback: "about" reads better than "introduction" for a page that is an overview of what AiiDA is. Isolated change; may be split off to a separate PR.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/source/tutorials/module1.md (1)
117-118: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winGuard the stdout parsing and failure path
node.exit_statusis printed, but the laterstdout_textparsing still assumes success and matching regexes. A failed run or format change will raise on.group(1)instead of a clearer error. Add a failure check and validate both regex matches before extracting the scalars; the TODO still leaves the failure case unaddressed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/source/tutorials/module1.md` around lines 117 - 118, Update the tutorial’s node result handling around the Process PK and exit-status prints to check for a failed run before parsing stdout. Validate both stdout regex matches before calling group(1), and raise a clear error for failed execution or unexpected output instead of allowing a match failure to surface.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/source/tutorials/module1.md`:
- Around line 312-315: Replace the shell-based !tree command in the module1
tutorial with a pathlib-based Python directory-tree display so myst-nb execution
does not depend on an undeclared OS package; preserve the existing purpose of
showing tmp/dump contents.
---
Outside diff comments:
In `@docs/source/tutorials/module1.md`:
- Around line 117-118: Update the tutorial’s node result handling around the
Process PK and exit-status prints to check for a failed run before parsing
stdout. Validate both stdout regex matches before calling group(1), and raise a
clear error for failed execution or unexpected output instead of allowing a
match failure to surface.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 36df41f5-a05a-4c6c-9e01-ff446d7c03f2
📒 Files selected for processing (2)
docs/source/tutorials/module1.mddocs/source/tutorials/module2.md
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/source/tutorials/module2.md
| ```{code-cell} ipython3 | ||
| # Show the directory tree of the dumped calculation data. | ||
| !tree tmp/dump | ||
| ``` |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== File list ==\n'
git ls-files docs/source/tutorials/module1.md docs/source | sed -n '1,120p'
printf '\n== Locate mentions of tree ==\n'
rg -n --hidden --no-messages '\btree\b' docs/source .github README* pyproject.toml setup.py setup.cfg requirements* || true
printf '\n== Tutorial outline ==\n'
ast-grep outline docs/source/tutorials/module1.md --view expanded || true
printf '\n== Relevant lines around 312-315 ==\n'
sed -n '280,340p' docs/source/tutorials/module1.mdRepository: aiidateam/aiida-core
Length of output: 13533
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check whether the docs/tutorial explicitly document any system dependency
# or alternative for listing directories, and whether the docs build uses a
# notebook execution environment that likely lacks OS utilities.
printf '\n== Relevant docs files ==\n'
git ls-files 'docs/**' '.github/**' 'README*' | sed -n '1,200p'
printf '\n== tree mentions with context ==\n'
rg -n -C 2 --hidden --no-messages '\btree\b' docs .github README* || true
printf '\n== Build/config mentions of notebook execution ==\n'
rg -n -C 2 --hidden --no-messages 'jupyter|notebook|myst|sphinx|execute|nbsphinx|jupyter-book|sphinx-gallery' docs .github pyproject.toml setup.cfg setup.py tox.ini .github/workflows || trueRepository: aiidateam/aiida-core
Length of output: 50380
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== myst-nb / notebook execution config ==\n'
sed -n '1,260p' docs/source/conf.py | rg -n -C 2 'myst|nb|jupyter|execute|notebook|ipynb|execution' || true
printf '\n== docs build workflow excerpt ==\n'
sed -n '1,220p' .github/workflows/docs-build.yml | rg -n -C 3 'tree|apt|brew|yum|myst|nb|jupyter|notebook|sphinx|install|ubuntu|rtd|read the docs' || true
printf '\n== docs dependency declarations ==\n'
sed -n '220,250p' pyproject.tomlRepository: aiidateam/aiida-core
Length of output: 5744
Avoid !tree in docs/source/tutorials/module1.md. The notebook is executed by myst-nb, but the docs setup only declares Python deps and the build workflow installs graphviz, not tree. Use pathlib here or document the OS package requirement.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/source/tutorials/module1.md` around lines 312 - 315, Replace the
shell-based !tree command in the module1 tutorial with a pathlib-based Python
directory-tree display so myst-nb execution does not depend on an undeclared OS
package; preserve the existing purpose of showing tmp/dump contents.
Applies the structural half of the group-meeting feedback across the
tutorial modules, leaving general wording and code simplification for a
later dedicated pass.
Splits (each keeps a single home-page card, so the index does not grow
extra panels; `tutorial:module3` and `tutorial:module6` are aliased onto
the "a" halves so existing cross-references still resolve):
- Module 3 into 3a (writing a workflow) and 3b (running it over many
inputs with `Map`).
- Module 6 into 6a (If / While) and 6b (dynamic shape and the adaptive
sweep).
Shared code (`gray_scott_pipeline`, `make_transition_plot`,
`fft_peak_wavelength`, the reused `If`-gated pipeline) stays in
`include/` as the single source of truth and is shown, folded, via a
`{literalinclude}` or a folded cell in each module that reuses it, so
nothing is hidden and nothing is duplicated. The small constants
(`BASE_PARAMS`, `F_VALUES`, the regexes) are inlined where used, kept
UPPERCASE since they are still constants and match `constants.py`.
Per module:
- M1/M2: inlined constants and regexes to UPPERCASE (consistency).
- M4: inline regexes, install note, drop `.resolve()`, reword
"byte-for-byte identical" to "exactly the same", point the sweep
reference to 3b.
- M5: adopt the new QueryBuilder filter syntax
(`orm.<Type>.fields.<name> <op> value`, extras via
`.fields.extras['key']`); the older dict form still works and is
noted. Inline the regex; install note.
- M7: install note (`aiida-core`, not the old `aiida` meta-package);
link aiida-qe-demo from "where to go next"; show the pipeline folded.
Each module gets a short install note at the top. Verified with isolated
notebook builds that treat execution errors as failures (M4 excepted: it
needs the SLURM container).
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/source/tutorials/module4.md`:
- Line 456: Revise the WorkChain claim in the tutorial text to qualify that the
swap applies only to workflows accepting a code/command input and forwarding it
to child calculations, such as gray_scott_sweep. Do not imply that it works for
arbitrary WorkChain implementations that hard-code the code or computer.
- Line 398: Update the `input_path` value passed to `launch_shell_job` in the
module 4 tutorial so `nodes['input']` uses an AiiDA `orm.SinglefileData` node
constructed from `input_path` rather than the raw path string.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 431b1ea6-d17e-48ee-98cc-305fad11dcbb
📒 Files selected for processing (10)
docs/source/tutorials/index.mddocs/source/tutorials/module1.mddocs/source/tutorials/module2.mddocs/source/tutorials/module3a.mddocs/source/tutorials/module3b.mddocs/source/tutorials/module4.mddocs/source/tutorials/module5.mddocs/source/tutorials/module6a.mddocs/source/tutorials/module6b.mddocs/source/tutorials/module7.md
🚧 Files skipped from review as they are similar to previous changes (4)
- docs/source/tutorials/module5.md
- docs/source/tutorials/module2.md
- docs/source/tutorials/module7.md
- docs/source/tutorials/module1.md
|
|
||
| from aiida_shell import launch_shell_job | ||
|
|
||
| input_path = 'include/input.yaml' |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
python - <<'PY'
from aiida_shell.calculations.shell import ShellJob
print(ShellJob.spec().inputs['nodes'].valid_type)
PYRepository: aiidateam/aiida-core
Length of output: 282
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Inspect the tutorial section around the reported lines.
sed -n '360,470p' docs/source/tutorials/module4.md
# Find nearby mentions of nodes/input_path/SinglefileData in the same tutorial.
rg -n "input_path|SinglefileData|nodes=|launch_shell_job|remote code|WorkChain|code switching" docs/source/tutorials/module4.mdRepository: aiidateam/aiida-core
Length of output: 5969
🌐 Web query:
aiida-shell launch_shell_job nodes input path string SinglefileData documentation
💡 Result:
In the aiida-shell library, the launch_shell_job function is designed to run shell commands while integrating inputs and outputs into the AiiDA provenance graph [1][2]. To pass files to a shell command, you use the nodes argument, which accepts a dictionary of AiiDA data nodes, most commonly SinglefileData [3]. 1. Preparing Input Files: You can create a SinglefileData node from a local file path or from a string [3][4]. Creating from a string is useful for scripts or configuration files generated on the fly [3][5]: from aiida.orm import SinglefileData # From string node_from_string = SinglefileData.from_string('content', filename='input.txt') # From path node_from_path = SinglefileData(file='/path/to/local/file.txt') 2. Launching the Job: Pass the SinglefileData nodes into the nodes dictionary [3]. Use placeholders in the arguments string (e.g., {key}) to specify where the filenames should appear in the command line [3]. from aiida_shell import launch_shell_job results, node = launch_shell_job( 'cat', arguments='{input_file}', nodes={ 'input_file': SinglefileData.from_string('hello world', filename='data.txt') }) 3. File Naming: By default, aiida-shell uses the key from the nodes dictionary as the filename in the working directory [3]. If you need a specific filename (e.g., to include a file extension), ensure it is set in the SinglefileData node's filename property during creation, or use the filenames argument in launch_shell_job to map specific nodes to custom file/directory names in the execution environment [3][5].
Citations:
- 1: https://aiida-shell.readthedocs.io/en/latest/
- 2: https://aiida-shell.readthedocs.io/
- 3: https://aiida-shell.readthedocs.io/en/latest/howto.html
- 4: https://aiida.readthedocs.io/projects/aiida-core/en/stable/topics/data_types.html
- 5: https://aiida-shell.readthedocs.io/en/latest/examples/qe.html
Pass a data node here instead of a raw path string. In docs/source/tutorials/module4.md:398, nodes={'input': input_path} will fail validation because launch_shell_job expects AiiDA Data nodes; use orm.SinglefileData(file=input_path) instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/source/tutorials/module4.md` at line 398, Update the `input_path` value
passed to `launch_shell_job` in the module 4 tutorial so `nodes['input']` uses
an AiiDA `orm.SinglefileData` node constructed from `input_path` rather than the
raw path string.
Source: MCP tools
| print(f"mean(V) = {mean_v:.4e}") | ||
| ``` | ||
|
|
||
| The same swap also isn't specific to `launch_shell_job`: it works for the pipeline from {ref}`Module 2 <tutorial:module2>`, the `gray_scott_sweep` workflow from {ref}`Module 3b <tutorial:module3b>`, and any {ref}`CalcJob <topics:calculations:concepts:calcjobs>` or {ref}`WorkChain <topics:workflows>` you may write in the future. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
set -euo pipefail
printf '\n## module4 excerpt\n'
sed -n '430,475p' docs/source/tutorials/module4.md
printf '\n## locate module2 and module3b references\n'
rg -n "gray_scott_sweep|launch_shell_job|code|computer" docs/source/tutorials -g '*.md' -g '*.py' -g '*.ipynb' || true
printf '\n## file list hints for tutorial sources\n'
git ls-files docs/source/tutorials | sed -n '1,200p'Repository: aiidateam/aiida-core
Length of output: 50378
🏁 Script executed:
set -euo pipefail
printf '\n## basic.md WorkChain example\n'
sed -n '455,505p' docs/source/tutorials/basic.md
printf '\n## module3b workflow signature\n'
sed -n '129,170p' docs/source/tutorials/module3b.md
printf '\n## nearby module4 wording\n'
sed -n '448,462p' docs/source/tutorials/module4.mdRepository: aiidateam/aiida-core
Length of output: 4325
Qualify the WorkChain claim. This swap works for workflows that accept a code/command input and pass it through to child calculations, like gray_scott_sweep; arbitrary WorkChain implementations may hard-code the code or computer.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/source/tutorials/module4.md` at line 456, Revise the WorkChain claim in
the tutorial text to qualify that the swap applies only to workflows accepting a
code/command input and forwarding it to child calculations, such as
gray_scott_sweep. Do not imply that it works for arbitrary WorkChain
implementations that hard-code the code or computer.
Source: MCP tools
The notebooks wrote gsrd runs and `verdi process dump` output to a local `tmp/` under the docs source tree. Sphinx scans the whole source tree for documents, so it picked up the dump's `tmp/dump/README.md` and warned it was not in any toctree. Write the scratch to `/tmp/aiida-tutorial` instead, so it stays out of the source tree entirely. Drop the now-unneeded gitignore entry.
The zmq -> zeromq broker refactor renamed the public broker class, its entry point, and its status API. include/setup_tutorial.py still used the old names, so it raised during import and profile creation, leaving gsrd_code undefined and breaking every module's notebook execution both locally and on ReadTheDocs. - ZmqBroker -> ZeromqBroker (import and isinstance check) - broker entry point core.zmq -> core.zeromq - broker.is_running -> broker.check_service_reachable()
Readers who copy-paste cells into their own notebook, rather than cloning the repo, had no include/ folder, so setup_tutorial.py, input.yaml and the shared task/workflow modules were missing and every notebook failed on its first include/ reference. Each module's setup cell now downloads setup_tutorial.py when it is absent; setup_tutorial.py then fetches the remaining include/ files via the GitHub contents API. Both checks are guarded, so a repo clone and the docs build make no network call. Module 0, which has no setup cell, fetches its two example inputs directly. The source ref is pinned to the PR branch for now (TODO in setup_tutorial.py); switch it to the release tag on merge.
3 participants
Rendered view here:
https://aiida--7205.org.readthedocs.build/projects/aiida-core/en/7205/tutorials/index.html
Warning
If RTD build in CI failed, the rendered view I link here is outdated!!
The important source files to review are:
docs/source/tutorials/module*.mdSummary by CodeRabbit
New Features
Documentation
Bug Fixes
%verdicommands now expand shell variables correctly.