Update tutorial 2 and add it to index.rst#40
Conversation
WalkthroughThis PR adds MoS2 and hBN example datasets and configurations (structures, training inputs, NEGF configs, model params), updates a graphene NEGF structure path, extends docs TOC with an extra tutorial, and adjusts .gitignore to new examples/ paths and hBN outputs. Most changes are data/config additions. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
.gitignore (1)
31-36: Wildcard the NEGF self-energy ignores for future tutorialsEnumerating every
(material)/(k-point)pair will keep biting us as we add more transport examples (e.g., the new MoS₂ flow). A single wildcarded pattern will cover all existing graphene/hBN artefacts and whatever comes next, without having to touch.gitignoreagain.-examples/graphene/negf_output_k20/self_energy/self_energy_leadL.h5 -examples/graphene/negf_output_k20/self_energy/self_energy_leadR.h5 -examples/hBN/negf_output_k20/self_energy/self_energy_leadL.h5 -examples/hBN/negf_output_k20/self_energy/self_energy_leadR.h5 -examples/hBN/negf_output_k50/self_energy/* -examples/hBN/negf_output_k70/self_energy/* +examples/*/negf_output_k*/self_energy/*
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
.gitignore(1 hunks)docs/hands_on/index.rst(1 hunks)examples/MoS2/MoS2_orth.vasp(1 hunks)examples/MoS2/POSCAR(1 hunks)examples/MoS2/extra_baseline/MoS_spds.json(1 hunks)examples/MoS2/extra_baseline/input_templete.json(1 hunks)examples/MoS2/extra_baseline/mos_spd_model/sktb.json(1 hunks)examples/MoS2/negf.json(1 hunks)examples/MoS2/orth_ckpt_r_max_5.3_sk.json(1 hunks)examples/MoS2/stru_negf.xyz(1 hunks)examples/MoS2/train/data/POSCAR(1 hunks)examples/MoS2/train/data/kpath.0/info.json(1 hunks)examples/MoS2/train/input.json(3 hunks)examples/MoS2/train/train_out/checkpoint/nnsk.best.pth(1 hunks)examples/graphene/negf.json(1 hunks)examples/hBN/negf.json(1 hunks)examples/hBN/siesta.TBT.AVTRANS_Left-Right(1 hunks)examples/hBN/stru_negf.xyz(1 hunks)examples/hBN/train/train_out/checkpoint/nnsk.best.pth(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: run-tests
🔇 Additional comments (11)
examples/MoS2/train/data/kpath.0/info.json (1)
1-12: MoS₂ k-path metadata looks goodValues line up with the single-frame dataset; no issues spotted.
examples/hBN/siesta.TBT.AVTRANS_Left-Right (1)
1-379: Transmission table format matches existing examplesHeader and two-column numeric layout mirror the other transport datasets; nothing blocking here.
examples/hBN/stru_negf.xyz (1)
1-123: Structure dataset checkedAtom count and lattice metadata look consistent for the segmented NEGF setup.
examples/MoS2/train/input.json (1)
52-58: Hopping kernel update aligns with SPD baselineSwitching the training config to
poly4powwith the same pairwisersvalues introduced in the SPD checkpoint keeps training and inference consistent across the new MoS₂ assets. Looks good.examples/MoS2/orth_ckpt_r_max_5.3_sk.json (1)
1-507: Checkpoint parameters look consistentSpot-checked orbitals, onsite, and hopping blocks; they align with the declared basis (Mo/S/W) and expected coefficient counts for
poly2pow. No concerns.examples/MoS2/extra_baseline/MoS_spds.json (1)
1-16: Baseline basis config is soundMo and S entries match the MoS₂ setup used elsewhere in the PR. Nothing to fix.
examples/MoS2/stru_negf.xyz (1)
1-75: NEGF structure data checks outAtom count (72) and lattice/coordinate formatting look correct for the XYZ schema. Good to go.
examples/MoS2/POSCAR (1)
1-11: POSCAR structure verifiedLattice vectors and three atomic positions line up with the MoS₂ primitive cell expectations. All good.
examples/MoS2/train/data/POSCAR (1)
1-16: Training POSCAR+velocities look fineCoordinates (with selective dynamics flags) plus zeroed Cartesian velocity block follow VASP format—no issues spotted.
examples/MoS2/MoS2_orth.vasp (1)
1-14: Orthorhombic cell data LGTMChecked lattice vectors, species counts, and Cartesian positions; everything is coherent for the enlarged MoS₂ cell.
examples/MoS2/extra_baseline/mos_spd_model/sktb.json (1)
1-613: spd model payload validatedCoefficient lengths match
poly4powexpectations, overlap data is present, and basis/device settings are consistent with the extra baseline. Looks good.
|
|
||
| .. toctree:: | ||
| tutorial1_c_chain | ||
| tutorial2_2d_mat |
There was a problem hiding this comment.
🧩 Analysis chain
Confirm toctree target exists
Please confirm that docs/hands_on/tutorial2_2d_mat.rst (or matching path) is present; otherwise the Sphinx build will emit a “document not found” warning.
Run this to double-check the file is there:
🏁 Script executed:
#!/bin/bash
fd tutorial2_2d_mat.rst docsLength of output: 0
🏁 Script executed:
#!/bin/bash
set -e
echo "Listing .rst files in docs/hands_on:"
fd -e rst docs/hands_on
echo
echo "Searching for 'tutorial2_2d_mat' in all .rst under docs:"
rg -l 'tutorial2_2d_mat' -g '*.rst' docs || trueLength of output: 394
🏁 Script executed:
#!/bin/bash
set -e
echo "Listing all .rst files in docs/hands_on:"
find docs/hands_on -maxdepth 1 -type f -name '*.rst' || echo "No .rst files found in docs/hands_on"
echo
echo "Searching for tutorial2_2d_mat.rst anywhere under docs:"
find docs -type f -name 'tutorial2_2d_mat.rst' || echo "tutorial2_2d_mat.rst not found"Length of output: 121
Confirm toctree target exists
docs/hands_on/tutorial2_2d_mat.rst not found; add the missing file or update the toctree entry at docs/hands_on/index.rst:7 to reference an existing document.
🤖 Prompt for AI Agents
In docs/hands_on/index.rst around line 7 the toctree entry "tutorial2_2d_mat"
points to a non-existent file; either add a new file at
docs/hands_on/tutorial2_2d_mat.rst with the intended content, or update the
toctree entry to reference the correct existing document name/path (matching the
actual .rst filename without extension). After updating, ensure the referenced
file is committed and run a local Sphinx build to confirm the toctree resolves.
| @@ -1,5 +1,5 @@ | |||
| { | |||
| "structure":"./struct.xyz", | |||
| "structure":"./struct_negf.xyz", | |||
There was a problem hiding this comment.
❓ Verification inconclusive
Confirm the new structure filename exists
Switching to struct_negf.xyz requires that file to be present; I don't see it in this PR. Please confirm it's checked in (or adjust the path) so runs don't fail loading the structure.
🏁 Script executed:
#!/bin/bash
# Verify the structure file path exists alongside the config.
fd --max-depth 1 struct_negf.xyz examples/grapheneLength of output: 0
Confirm or add struct_negf.xyz alongside the config
examples/graphene/negf.json:2 – the referenced structure file ./struct_negf.xyz is missing in this PR. Please check in struct_negf.xyz (or correct the path) so loading the structure won’t fail.
🤖 Prompt for AI Agents
In examples/graphene/negf.json around line 2, the config references
"./struct_negf.xyz" which is not present in the PR; either add the missing
struct_negf.xyz file next to negf.json (with the expected XYZ content and
correct filename) or update the "structure" path in negf.json to point to the
correct existing file location; ensure the path is relative to examples/graphene
and that the checked-in file uses the expected XYZ format so structure loading
will succeed.
| @@ -0,0 +1,53 @@ | |||
| { | |||
| "structure":"./struct_negf.xyz", | |||
There was a problem hiding this comment.
Fix the structure path typo
The config points to struct_negf.xyz, but the new hBN structure file in this PR is named stru_negf.xyz. With the current path the run will fail to load the structure.
Apply this diff to align the path with the committed filename:
- "structure":"./struct_negf.xyz",
+ "structure":"./stru_negf.xyz",📝 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.
| "structure":"./struct_negf.xyz", | |
| "structure":"./stru_negf.xyz", |
🤖 Prompt for AI Agents
In examples/hBN/negf.json around line 2, the "structure" value points to
"./struct_negf.xyz" but the committed file is named "stru_negf.xyz"; update the
path to "./stru_negf.xyz" so the config matches the actual filename and the
structure file can be loaded.
| @@ -0,0 +1 @@ | |||
| /personal/DeepTB/dptb_Zjj/dpnegf/examples/hBN/train/train_out/checkpoint/nnsk.ep3000.pth No newline at end of file | |||
There was a problem hiding this comment.
Broken checkpoint path committed
This file is a symlink pointing to /personal/DeepTB/dptb_Zjj/dpnegf/examples/hBN/train/train_out/checkpoint/nnsk.ep3000.pth, which only exists in the author’s local environment. Anyone pulling the repo will resolve the symlink to a nonexistent path, so the “best” checkpoint can’t be loaded and the hBN example breaks. Please replace this with an actual checkpoint in-repo (if its size/policy allows) or use a relative path within the tree so the asset resolves everywhere.
🤖 Prompt for AI Agents
In examples/hBN/train/train_out/checkpoint/nnsk.best.pth (line 1), the committed
file is a symlink pointing to an absolute local path that doesn't exist for
other users; replace the broken absolute symlink by either: 1) committing the
actual checkpoint file at examples/hBN/train/train_out/checkpoint/nnsk.best.pth
(if repo policy and size allow), or 2) changing the symlink to point to a
relative path inside the repository that resolves for all users, or 3) remove
the symlink and add a small script/README that downloads the checkpoint into the
checkpoint directory during setup; ensure the final file or link resolves to a
path present in the repo or can be reproducibly fetched by contributors.
| "train": { | ||
| "root": "path/to/dataset", | ||
| "prefix": "prexfix_for_dataset", | ||
| "get_eigenvalues": true, | ||
| "type": "DefaultDataset", | ||
| "get_Hamiltonian": false | ||
| }, | ||
| "validation": { | ||
| "root": "path/to/dataset", | ||
| "prefix": "prexfix_for_dataset", | ||
| "get_eigenvalues": true, | ||
| "type": "DefaultDataset", | ||
| "get_Hamiltonian": false | ||
| }, | ||
| "reference": { | ||
| "root": "path/to/dataset", | ||
| "prefix": "prexfix_for_dataset", | ||
| "get_eigenvalues": true, | ||
| "type": "DefaultDataset", | ||
| "get_Hamiltonian": false | ||
| } |
There was a problem hiding this comment.
Point the template to the committed MoS₂ dataset
With root still set to "path/to/dataset" and the placeholder "prexfix_for_dataset", anyone trying to run this “baseline” straight from the repo will immediately hit a FileNotFoundError because the loader can’t locate the data you added under examples/MoS2/train/data. Let’s wire the template to that dataset (both for train/val/reference) so the example actually runs out of the box.
- "root": "path/to/dataset",
- "prefix": "prexfix_for_dataset",
+ "root": "../train/data",
+ "prefix": "kpath.0",Repeat the same adjustment for the validation and reference sections.
📝 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.
| "train": { | |
| "root": "path/to/dataset", | |
| "prefix": "prexfix_for_dataset", | |
| "get_eigenvalues": true, | |
| "type": "DefaultDataset", | |
| "get_Hamiltonian": false | |
| }, | |
| "validation": { | |
| "root": "path/to/dataset", | |
| "prefix": "prexfix_for_dataset", | |
| "get_eigenvalues": true, | |
| "type": "DefaultDataset", | |
| "get_Hamiltonian": false | |
| }, | |
| "reference": { | |
| "root": "path/to/dataset", | |
| "prefix": "prexfix_for_dataset", | |
| "get_eigenvalues": true, | |
| "type": "DefaultDataset", | |
| "get_Hamiltonian": false | |
| } | |
| "train": { | |
| "root": "../train/data", | |
| "prefix": "kpath.0", | |
| "get_eigenvalues": true, | |
| "type": "DefaultDataset", | |
| "get_Hamiltonian": false | |
| }, | |
| "validation": { | |
| "root": "../train/data", | |
| "prefix": "kpath.0", | |
| "get_eigenvalues": true, | |
| "type": "DefaultDataset", | |
| "get_Hamiltonian": false | |
| }, | |
| "reference": { | |
| "root": "../train/data", | |
| "prefix": "kpath.0", | |
| "get_eigenvalues": true, | |
| "type": "DefaultDataset", | |
| "get_Hamiltonian": false | |
| } |
🤖 Prompt for AI Agents
In examples/MoS2/extra_baseline/input_templete.json around lines 71-91 the
dataset paths are placeholders ("path/to/dataset" and "prexfix_for_dataset")
which will cause FileNotFoundError; update the "root" values to point to the
committed example dataset directory (examples/MoS2/train/data) and set the
"prefix" to the actual prefix used by those files for the train, validation, and
reference sections so the example runs out-of-the-box.
| { | ||
| "structure":"./struct_negf.xyz", |
There was a problem hiding this comment.
Fix the structure filename mismatch.
The NEGF config points to struct_negf.xyz, but the file added in this PR is stru_negf.xyz. With the current path the solver will fail immediately when trying to load the structure.
Apply this diff to correct the reference:
- "structure":"./struct_negf.xyz",
+ "structure":"./stru_negf.xyz",📝 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.
| { | |
| "structure":"./struct_negf.xyz", | |
| { | |
| "structure":"./stru_negf.xyz", |
🤖 Prompt for AI Agents
In examples/MoS2/negf.json around lines 1-2 the "structure" value points to
"./struct_negf.xyz" which doesn't match the file added in this PR
("stru_negf.xyz"); update the JSON to reference the correct filename (e.g.
"./stru_negf.xyz") so the solver can find and load the structure.
| @@ -0,0 +1 @@ | |||
| /personal/DeepTB/dptb_Zjj/dpnegf/examples/MoS2/train/train_out/checkpoint/nnsk.ep3000.pth No newline at end of file | |||
There was a problem hiding this comment.
Replace hardcoded absolute checkpoint path
This file currently contains a user-specific absolute path instead of an actual checkpoint or a portable relative reference, so the bundled MoS₂ workflow can’t load a model anywhere else. Please either drop the file or replace it with the real checkpoint (or a relative placeholder plus docs). Until then, the example remains broken for everyone else.
🤖 Prompt for AI Agents
In examples/MoS2/train/train_out/checkpoint/nnsk.best.pth (lines 1-1), the file
contains a user-specific absolute checkpoint path which breaks portability;
either remove this file from the repo, or replace its contents with a real
checkpoint binary (or a relative placeholder file name) and update the example
README to document where to place or how to generate the checkpoint (include
expected relative path and load instructions). Ensure the committed path is
relative (or the actual .pth blob) and update any code that references the
absolute path to use that relative path or a configurable environment variable.
1 participant
This pull request adds comprehensive support for MoS₂ (Molybdenum disulfide) examples, including training, data, and NEGF (Non-Equilibrium Green's Function) calculations. It introduces new configuration and structure files, updates documentation, and adapts input templates for MoS₂. The changes are grouped below by theme.
MoS₂ Example Addition and Configuration
MoS2_orth.vasp,POSCAR,stru_negf.xyz) and NEGF configuration (negf.json) for transport calculations. [1] [2] [3] [4]extra_baseline/input_templete.json,extra_baseline/MoS_spds.json), training data (train/data/POSCAR,train/data/kpath.0/info.json), and training output checkpoint (train/train_out/checkpoint/nnsk.best.pth). [1] [2] [3] [4] [5]Documentation Update
NEGF Configuration Updates
Summary by CodeRabbit
New Features
Documentation
Chores