feature/SOF-8044 Feature: add the h-BN boron-vacancy formation-energy Specific Example - #367
Draft
VsevolodX wants to merge 6 commits into
Draft
feature/SOF-8044 Feature: add the h-BN boron-vacancy formation-energy Specific Example#367VsevolodX wants to merge 6 commits into
VsevolodX wants to merge 6 commits into
Conversation
…on-energy SE The defect_formation_energy workflow needs the pristine and defective supercells as a pair; the structure notebook only named and saved the defective one. Name and save the pristine supercell too, and link the new simulation notebook from Introduction.ipynb's D-0D-VAC row (SOF-8044).
Computes the neutral formation energy of the boron vacancy created in
defect_point_vacancy_boron_nitride.ipynb with the defect_formation_energy workflow
and compares it against QPOD's v_B in BN (charge 0) entry (Bertoldo et al. 2022).
Elemental and pristine Total Energy references are computed under this account with
the same PBE/PAW model as the defect job (D2) rather than trusted from curators, whose
seeded values may use a different pseudopotential; the workflow's own elemental-total-
energy resolution otherwise defaults to any owner ('public'), so its source is patched
to this account. B and N have no ultrasoft PBE pseudopotential on the platform, so the
model uses PAW throughout, and the defective cell's SCF is patched to nspin=2 with an
initial magnetization for the spin-polarized vacancy. k-grids are derived per material
from the paper's own k-point density (SOF-8044).
…oads The workflow selects elemental materials by tags/metadata.element and then total_energy properties by those materials' exabyteId; an uploaded Standata copy gets a fresh exabyteId and can never match, so the notebook's own B/N Total Energy jobs were invisible to it (review blocker). Elemental references now come from client.materials.list, the same platform documents the workflow itself resolves; the Total Energy prerequisite jobs run directly on them. Also, per review: - reuse check now mirrors the workflow's own precondition (a finished job named Total Energy on the material, owned by this account) instead of a property lookup that could match a different workflow or a stale model; - errored/unfinished prerequisite or defect jobs raise before anything downstream is built or read; - starting_magnetization is set on both species (B, N) -- the V_B moment sits on the N neighbours, not on B; - model reverted to GBRV ultrasoft (40/200 Ry), which exists for B and N; PAW's only justification was matching QPOD's GPAW setup; k-point density comment corrected (QPOD: 6 for relaxations, 12 for ground states -- 6 used here for all jobs); - results cell recomputes E_f from the notebook's own three total energies and warns if it disagrees with the workflow's value by more than 1 meV.
…ross cutoffs
Round-2 review blocker: the reuse query for the elemental prerequisite jobs matched
any finished job named "Total Energy" on the material, regardless of what cutoffs or
pseudopotential it used -- a curators' total_energy at a different model would be
silently reused and pass the consistency check (which recomputes from that same job).
Fix: the reference workflow name now carries the exact model
("Total Energy <material> pbe-us 40/200 Ry"), and reuse matches that name exactly
(no regex), sorted by updatedAt desc, limit 1 -- still a superstring "Total Energy",
so fetch_bulk_total_energy's own regex still finds the pristine's job.
Also, per review:
- mu_B (per atom) and the B reference's atom count are printed, both where the
elemental references are resolved and in the results cell;
- the reference-mismatch warning now carries into the verdict line instead of
sitting two cells above it;
- the three total_energy property lookups in the results cell raise a named
RuntimeError instead of a bare IndexError on an empty list;
- an unmatched CLUSTER_NAME raises, listing the available cluster hostnames,
instead of silently falling back to an arbitrary cluster;
- the defect workflow's k-grid is applied with apply_scf_kgrid, like every other
workflow in this notebook, instead of a second hand-built context;
- atom count is read via basis.number_of_atoms everywhere (was mixed with
len(...elements.ids)); the redundant second sorted() on elements is dropped;
the k-point density comment states why 6, not 12 (cost; the neutral E_f does
not need it).
…nale Round-3 review blocker: cell 32's cluster lookup did `CLUSTER_NAME in c["hostname"]` unconditionally, which TypeErrors on None -- so a harness (or a user) could not point the notebook at "whatever cluster is available" the way every other SE in this directory allows. Restored the sibling's shape: raise (listing hostnames) when CLUSTER_NAME is set and not found, fall back to the first listed cluster when it is None. The prerequisite-reuse comment cited "fetch_bulk_total_energy's regex" for the pristine; the installed workflow (and standata main) resolves E_PRISTINE by a properties query with no job-name filter at all -- that regex form only exists in a local, behind-main checkout. Corrected the comment: the workflow name change is justified by the notebook's own reuse being model-aware, nothing else; if the pristine path still resolves a different reference, the results cell's consistency check catches it. Also: journal corrected to npj Computational Materials; the header states Delta N_B = -1 next to Eq. (3); ECUTWFC/ECUTRHO comments carry their units (Ry); Usage/Summary sections added, matching the other two SIMULATION notebooks; MODEL_TAG dropped its "/" (now "pbe-us 40-200Ry", which slugifies cleanly) and the defect workflow's name carries it too, for provenance; TIME_LIMIT is commented.
Round-4 review: the structure notebook's header still cited "Nature" for Bertoldo et al. (2022), which is npj Computational Materials -- the documentation page already says npj, so the two disagreed on one screen. Same fix already applied to the simulation notebook's header in an earlier round. The simulation notebook's new Usage block said the verdict reports "the recomputed E_f" -- it reports the workflow's own defect_formation_energy; the recomputed value is only the mismatch flag. Fixed the sentence.
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks 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 |
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Specific Example: the neutral boron-vacancy formation energy in monolayer h-BN, compared with the QPOD database (Bertoldo et al., npj Comput. Mater. 2022, entry
1BN-1.2d.v_B.0.1: 10.18 eV, standard-state chemical potentials). Jira SOF-8044, epic SOF-7994.What changed
specific_examples/defect_point_vacancy_boron_nitride.ipynb— the pristine supercell is now named (h-BN supercell) and saved next toB-vacancy h-BN, so the simulation notebook can load the pair by name.specific_examples/defect_point_vacancy_boron_nitride_SIMULATION.ipynb— loads the pair (raises on a missing name), resolves the elemental references from the platform's curatedelementalmaterials, runs Total Energy jobs for the pristine cell, α-boron and nitrogen with one model (PBE, GBRV ultrasoft, 40/200 Ry, k-point density 6 Å⁻¹) unless a job namedTotal Energy <material> pbe-us 40/200 Ryalready exists, then onedefect_formation_energyjob (nspin = 2,[defective, pristine]). The results cell recomputes E_f from the notebook's own three total energies beside the workflow's value and printsReproduces Bertoldo et al. (2022): yes|noat |ΔE_f| ≤ 0.5 eV.specific_examples/Introduction.ipynb— D-0D-VAC row gains its Simulation link.Verified
pw_scfinput carriesnspin = 2,starting_magnetization(1)/(2), 40/200 Ry; k-grids 3×5×1 (h-BN), 10×10×10 (B), 7×7×7 (N).rd3ddLJhyrR9EiLzF(pristine, 48 atoms, 3×5×1),bqmc3Xeof6dHXpHuT(α-B),noPrB8xQBbFe89wA6(N); defect jobA8LzXbBjYhXKNh6ip(47 atoms, nspin 2). Printed:Manual checks (merging accepts anything unticked)
pw_scfinput of the defect job:nspin = 2,starting_magnetization(1)and(2),ecutwfc = 40, and everyATOMIC_SPECIESline has a.upffilename.pw_scf.outreportstotal magnetization ≈ 1.00 Bohr mag/cell(QPOD: 1.018 μB, a doublet). Measured 3.00 μB — the SCF converged to the quartet; E_f still within 0.06 eV of QPOD. Whether the notebook should constrain the spin (tot_magnetization = 1) is open.reusinglines and creates nothing.Docs PR: mat3ra/documentation#404 (merge after this one — the embed resolves against the JupyterLite build of api-examples main).