Skip to content

fix: ELEMENTAL_TE_SOURCE was never applied to the submitted job - #353

Open
VsevolodX wants to merge 1 commit into
mainfrom
fix/formation-energy-te-source
Open

fix: ELEMENTAL_TE_SOURCE was never applied to the submitted job#353
VsevolodX wants to merge 1 commit into
mainfrom
fix/formation-energy-te-source

Conversation

@VsevolodX

@VsevolodX VsevolodX commented Aug 1, 2026

Copy link
Copy Markdown
Member

Problem

formation_energy.ipynb's ELEMENTAL_TE_SOURCE parameter (cell 1.3) was only ever used in cell 4.2's client-side preview print. formation_workflow -- the object actually passed to create_job -- was never patched with it, so the Resolve Total Energies for Elemental Materials subworkflow always ran with its standata-hardcoded 'public' value on the cluster, regardless of what the notebook parameter said. Preview and reality could silently disagree.

This was the source of intermittent formation_energy Cypress failures: public scope picks whichever account's total_energy for the same elemental structure currently has the highest recorded precision. Different accounts' default (unpinned) Total Energy jobs land on different auto-selected precision for the identical structure, so which one "wins" can permanently shift as new candidates enter the shared properties pool over time.

Fix

Patch the workflow's own assign-source-of-te-for-an-element unit right after the workflow object is created, mirroring how SCF_KGRID already patches pw_scf.

Verification

Confirmed locally end-to-end (local backend + a temporary Netlify preview of this branch): with this fix, plus pinning the elemental Si/Ge Total Energy jobs to an explicit k-grid in the Cypress test (a separate, already-existing gap in the test's Background), formation_energy for SiGe now resolves deterministically to 1.3642469947276368 eV/atom -- matching the original calibration -- across repeated runs.

Summary by CodeRabbit

  • Bug Fixes
    • Formation Energy workflows now use the selected elemental energy source when resolving elemental total energies at runtime.
    • Updated workflow configuration sequencing to ensure reliable execution.

… job

The notebook's cell 4.2 uses ELEMENTAL_TE_SOURCE to preview which
total_energy properties would be resolved, but formation_workflow (the
object actually passed to create_job) was never patched with it -- the
Resolve Total Energies for Elemental Materials subworkflow always ran
with its standata-hardcoded 'public' value regardless of the parameter,
so the preview and the real computation could silently disagree.

Patch the workflow's assign-source-of-te-for-an-element unit the same
way SCF_KGRID already patches pw_scf, right after the workflow object
is created and before it's saved/submitted.
@review-notebook-app

Copy link
Copy Markdown

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@coderabbitai

coderabbitai Bot commented Aug 1, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 5fcec5b2-7ab5-491c-b6fd-d57f95e13bf0

📥 Commits

Reviewing files that changed from the base of the PR and between fd8fc52 and 300f11f.

📒 Files selected for processing (1)
  • other/materials_designer/workflows/formation_energy.ipynb

📝 Walkthrough

Walkthrough

The Formation Energy notebook now applies ELEMENTAL_TE_SOURCE when resolving elemental total energies before workflow submission. Subsequent notebook cell IDs were incremented. Other cell contents and execution behavior remain unchanged.

Changes

Formation Energy workflow

Layer / File(s) Summary
Runtime source configuration and notebook sequencing
other/materials_designer/workflows/formation_energy.ipynb
The workflow configuration sets the elemental total-energy source from ELEMENTAL_TE_SOURCE before saving and submitting. Later cell IDs were incremented without changing their contents or execution behavior.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

  • mat3ra/api-examples#343: Modifies the same Formation Energy notebook and adds runtime handling for ELEMENTAL_TE_SOURCE.
  • mat3ra/api-examples#352: Modifies elemental total-energy resolution and source-variable handling in the Formation Energy workflow.

Suggested reviewers: timurbazhirov

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the main fix: applying ELEMENTAL_TE_SOURCE to the submitted job.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/formation-energy-te-source

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant