SOF-7777 Update: add python code to get VBO polar #265
Conversation
📝 WalkthroughWalkthroughAdds six Python package requirements and a new workflow type-stub/script Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User as User
participant Script as vbo_polar.pyi
participant Reader as StructureReader
participant Checkpoint as CheckpointLoader
participant Fitter as FittingRoutine
participant Plotter as Plotter
rect rgba(200,200,255,0.5)
User->>Script: invoke with 3 structure files + checkpoint
end
rect rgba(200,255,200,0.5)
Script->>Reader: load three pw_scf.out-like structures
Reader-->>Script: atomic positions & cell (cartesian)
Script->>Script: detect slab z-boundaries
end
rect rgba(255,200,200,0.5)
Script->>Checkpoint: load macroscopic potential profiles (X,Y)
Checkpoint-->>Script: X/Y for interface, bulk_left, bulk_right
end
rect rgba(255,230,200,0.5)
Script->>Fitter: call get_region_indices & fit_and_average for regions
Fitter-->>Script: region indices, averages, slopes, intercepts
Script->>Script: compute Va/Vb and VBO
end
rect rgba(200,255,255,0.5)
Script->>Plotter: draw profile, fits, regions
Plotter-->>Script: saved PNG
Script-->>User: print VBO and file path
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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: 2
🤖 Fix all issues with AI agents
In `@other/materials_designer/workflows/requirements-with-made.txt`:
- Around line 1-5: Update the mat3ra-made version constraint in
requirements-with-made.txt so it references a real PyPI release instead of the
non-existent "2024.11.12.0"; replace the current "mat3ra-made >= 2024.11.12.0"
spec with a valid release such as "mat3ra-made >= 2024.11.26.post0" (or another
listed existing release like 2024.11.23.post0) to ensure the package can be
installed.
In `@other/materials_designer/workflows/vbo_polar.pyi`:
- Around line 61-67: The helper get_region_indices currently returns (0,
len(x_data)) when no points fall in [x_min, x_max], which silently causes
full-profile fits; change get_region_indices to fail fast by raising a
ValueError with a clear message including x_min and x_max (e.g., "empty region
for x_min=..., x_max=...") instead of returning a full-range sentinel; apply the
same change to the other identical implementation around lines 94-100 so callers
cannot accidentally fit the entire profile when a region is empty.
| munch == 2.5.0 | ||
| numpy >= 1.19.5 | ||
| scipy >= 1.5.4 | ||
| matplotlib >= 3.0.0 | ||
| mat3ra-made >= 2024.11.12.0 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
python - <<'PY'
import json, urllib.request
pkgs = {
"munch": "2.5.0",
"numpy": "1.19.5",
"scipy": "1.5.4",
"matplotlib": "3.0.0",
"mat3ra-made": "2024.11.12.0",
}
for pkg, min_ver in pkgs.items():
url = f"https://pypi.org/pypi/{pkg}/json"
try:
data = json.load(urllib.request.urlopen(url))
except Exception as e:
print(f"{pkg}: unable to query PyPI ({e})")
continue
latest = data["info"]["version"]
exists = min_ver in data.get("releases", {})
print(f"{pkg}: latest={latest}, min_spec={min_ver}, exists_on_pypi={exists}")
PYRepository: Exabyte-io/api-examples
Length of output: 153
🌐 Web query:
mat3ra-made PyPI package 2024.11.12.0
💡 Result:
I couldn't find a PyPI release named exactly "mat3ra-made 2024.11.12.0". The mat3ra-made PyPI project page and piwheels release list show multiple November 2024 releases (e.g. 2024.11.1.post0, 2024.11.6.post0, 2024.11.7.post0, 2024.11.16.post0, 2024.11.19.post0, 2024.11.23.post0, 2024.11.26.post0) but not 2024.11.12.0. [1][2]
Sources:
- mat3ra-made on PyPI. [1]
- mat3ra-made release list on piwheels. [2]
🌐 Web query:
numpy 1.19.5 scipy 1.5.4 matplotlib 3.0.0 compatibility
💡 Result:
Short answer: compatible.
- SciPy 1.5.x requires NumPy >=1.14.5 and <1.20.0, so NumPy 1.19.5 is within the supported range (SciPy 1.5.4 will work with NumPy 1.19.5). [1]
- Matplotlib 3.0.0 has a very low NumPy minimum (>=1.10.0), so NumPy 1.19.5 is fine for Matplotlib 3.0.0. [2]
Note: SciPy 1.5.x was targeted at Python 3.6–3.9-era runtimes, so ensure your Python version is also compatible with those package versions. [1]
Sources:
[1] SciPy toolchain / version support (SciPy 1.5: NumPy >=1.14.5, <1.20.0).
[2] Matplotlib minimum dependency table (Matplotlib 3.0: NumPy >=1.10.0).
🌐 Web query:
munch 2.5.0 PyPI
💡 Result:
munch 2.5.0 is a published PyPI release (uploaded Oct 30, 2019). You can install it with:
pip install munch==2.5.0. [1][2]
Sources:
[1] PyPI project page (release history).
[2] PyPI release listing for munch (shows 2.5.0, Oct 30, 2019).
Fix mat3ra-made version constraint—2024.11.12.0 does not exist on PyPI.
The specified version mat3ra-made >= 2024.11.12.0 cannot be installed; this version is not published on PyPI. Available November 2024 releases include 2024.11.1.post0, 2024.11.6.post0, 2024.11.7.post0, 2024.11.16.post0, 2024.11.19.post0, 2024.11.23.post0, and 2024.11.26.post0. Update the constraint to use an existing version.
The other dependencies (munch 2.5.0, numpy 1.19.5, scipy 1.5.4, matplotlib 3.0.0) all exist on PyPI and are mutually compatible.
🤖 Prompt for AI Agents
In `@other/materials_designer/workflows/requirements-with-made.txt` around lines 1
- 5, Update the mat3ra-made version constraint in requirements-with-made.txt so
it references a real PyPI release instead of the non-existent "2024.11.12.0";
replace the current "mat3ra-made >= 2024.11.12.0" spec with a valid release such
as "mat3ra-made >= 2024.11.26.post0" (or another listed existing release like
2024.11.23.post0) to ensure the package can be installed.
| def get_region_indices(x_data, x_min, x_max): | ||
| """Get array indices corresponding to coordinate range.""" | ||
| mask = (x_data >= x_min) & (x_data <= x_max) | ||
| indices = np.where(mask)[0] | ||
| if len(indices) == 0: | ||
| return 0, len(x_data) | ||
| return indices[0], indices[-1] + 1 |
There was a problem hiding this comment.
Don’t fit the entire profile when a region is empty.
Returning (0, len(x_data)) on empty regions silently fits the full profile and produces incorrect Va/Vb when the region bounds are wrong. Fail fast (or explicitly return empty) instead.
🐛 Proposed fix (fail fast on empty regions)
def get_region_indices(x_data, x_min, x_max):
"""Get array indices corresponding to coordinate range."""
mask = (x_data >= x_min) & (x_data <= x_max)
indices = np.where(mask)[0]
if len(indices) == 0:
- return 0, len(x_data)
+ return 0, 0
return indices[0], indices[-1] + 1
# Get indices for each slab region
slab1_start, slab1_end = get_region_indices(X, z_min_1, z_max_1)
slab2_start, slab2_end = get_region_indices(X, z_min_2, z_max_2)
+
+if slab1_end <= slab1_start:
+ raise ValueError(f"No points found in slab 1 region ({z_min_1}, {z_max_1}).")
+if slab2_end <= slab2_start:
+ raise ValueError(f"No points found in slab 2 region ({z_min_2}, {z_max_2}).")Also applies to: 94-100
🤖 Prompt for AI Agents
In `@other/materials_designer/workflows/vbo_polar.pyi` around lines 61 - 67, The
helper get_region_indices currently returns (0, len(x_data)) when no points fall
in [x_min, x_max], which silently causes full-profile fits; change
get_region_indices to fail fast by raising a ValueError with a clear message
including x_min and x_max (e.g., "empty region for x_min=..., x_max=...")
instead of returning a full-range sentinel; apply the same change to the other
identical implementation around lines 94-100 so callers cannot accidentally fit
the entire profile when a region is empty.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@other/materials_designer/workflows/vbo_polar.pyi`:
- Around line 95-107: In fit_and_average, do not silently return 0.0 for an
empty region; instead detect when x_region and y_region are empty and raise a
ValueError describing the empty fit region, and only use the "avg with zero
slope" fallback when the region contains exactly one point (compute avg =
float(np.mean(y_region)) and return avg, 0.0, avg); keep the regular linear fit
logic for len(x_region) >= 2. Ensure the error message mentions fit_and_average
and the start/end indices for debugging.
- Around line 1-79: This file is a runtime script (it performs imports, file I/O
and computes X/Y from profile_interface) but is saved as a type stub (.pyi);
rename the file from .pyi to .py so Python will execute it at runtime, and
ensure any consumers import the module by its new name; verify symbols like
Material.create, from_ase, ase.io.read and the variables pw_scf_output_1/2,
profile_interface, profile_left/right, and X/Y continue to work after renaming
and update any references or imports elsewhere to point to the new module name.
♻️ Duplicate comments (1)
other/materials_designer/workflows/vbo_polar.pyi (1)
87-93: Fail fast on empty regions instead of fitting the full profile.Returning
(0, len(x_data))on empty regions silently fits the entire profile and yields incorrect Va/Vb. Prefer raising a clear error when no points are found.🐛 Proposed fix (fail fast)
def get_region_indices(x_data, x_min, x_max): """Get array indices corresponding to coordinate range.""" mask = (x_data >= x_min) & (x_data <= x_max) indices = np.where(mask)[0] if len(indices) == 0: - return 0, len(x_data) + raise ValueError(f"empty region for x_min={x_min}, x_max={x_max}") return indices[0], indices[-1] + 1
| def fit_and_average(x_data, y_data, start_idx, end_idx): | ||
| """ | ||
| Fit linear regression to region and return average value, slope, and intercept. | ||
|
|
||
| The average of the fitted line equals the mean of y-values, | ||
| but fitting helps smooth out oscillations and validates linearity. | ||
| """ | ||
| x_region = x_data[start_idx:end_idx] | ||
| y_region = y_data[start_idx:end_idx] | ||
|
|
||
| if len(x_region) < 2: | ||
| avg = float(np.mean(y_region)) if len(y_region) > 0 else 0.0 | ||
| return avg, 0.0, avg |
There was a problem hiding this comment.
Avoid silent 0.0 when the fit region is empty.
Returning 0.0 for an empty region can silently skew the VBO. Consider raising a ValueError on empty regions and only using the “avg with zero slope” fallback for exactly one point.
🐛 Proposed fix (guard empty region)
def fit_and_average(x_data, y_data, start_idx, end_idx):
@@
x_region = x_data[start_idx:end_idx]
y_region = y_data[start_idx:end_idx]
- if len(x_region) < 2:
- avg = float(np.mean(y_region)) if len(y_region) > 0 else 0.0
- return avg, 0.0, avg
+ if len(x_region) == 0:
+ raise ValueError(f"empty fit region for indices [{start_idx}, {end_idx})")
+ if len(x_region) == 1:
+ avg = float(y_region[0])
+ return avg, 0.0, avg🤖 Prompt for AI Agents
In `@other/materials_designer/workflows/vbo_polar.pyi` around lines 95 - 107, In
fit_and_average, do not silently return 0.0 for an empty region; instead detect
when x_region and y_region are empty and raise a ValueError describing the empty
fit region, and only use the "avg with zero slope" fallback when the region
contains exactly one point (compute avg = float(np.mean(y_region)) and return
avg, 0.0, avg); keep the regular linear fit logic for len(x_region) >= 2. Ensure
the error message mentions fit_and_average and the start/end indices for
debugging.
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.