Skip to content

fix(mat-surface-adsorption): resolve site labels from matcalc adsorption_site - #50

Merged
bowen-bd merged 1 commit into
mainfrom
fix/49-adsorption-site-label
Oct 7, 2026
Merged

bowen-bd merged 1 commit into
mainfrom
fix/49-adsorption-site-label

Conversation

@bowen-bd

@bowen-bd bowen-bd commented Oct 7, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes #49 where site labels in adsorption_results.json and execution logs always reported "unknown" instead of the actual adsorption site names ("ontop", "bridge", "hollow", etc.).

Root Cause

In matcalc 0.5.1, AdsorptionCalc.calc_adslabs sets site metadata using the key "adsorption_site" (along with "adsorption_site_coord" and "adsorption_site_index"), whereas "site" is never set. In calculate_adsorption.py, the code looked up result.get("site", "unknown"), which always triggered the fallback.

Changes

  • Check adsorption_site first, with fallback to legacy site and default "unknown" in skills/mat-surface-adsorption/scripts/calculate_adsorption.py.
  • Record site_coord from adsorption_site_coord in the output JSON for traceability.
  • Extracted process_adsorption_results for modularity.
  • Added regression and contract tests in tests/test_mat_surface_adsorption_skill.py.

…ion_site

- Look up 'adsorption_site' before 'site' fallback in calculate_adsorption.py
- Preserve 'site_coord' from 'adsorption_site_coord' in output JSON
- Extract process_adsorption_results for clean testing
- Add unit/regression tests in tests/test_mat_surface_adsorption_skill.py

Fixes #49
@bowen-bd
bowen-bd merged commit 264983a into main Oct 7, 2026
23 checks passed
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.

mat-surface-adsorption: site labels show as "unknown" — result.get("site") should be result.get("adsorption_site")

1 participant