(5) Feature/SOF-7845 update: add Relaxation NB#277
Merged
Conversation
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
📝 WalkthroughWalkthroughThis PR introduces a new relaxation workflow notebook and updates the introduction to link to it. The new notebook implements a complete end-to-end workflow for relaxing material structures using Quantum ESPRESSO on the Mat3ra platform, covering environment setup, authentication, material handling, workflow selection, parameter configuration, job orchestration, monitoring, and results retrieval. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant Notebook as Jupyter Notebook
participant API as Mat3ra Platform API
participant FS as File System
participant Standata as Standata
participant QE as Quantum ESPRESSO
User->>Notebook: Setup environment & config
Notebook->>Notebook: Install dependencies
User->>Notebook: Provide credentials
Notebook->>API: Authenticate
API-->>Notebook: Auth token
Notebook->>FS: Load or retrieve material
Notebook->>API: Save material to platform
API-->>Notebook: Material ID
Notebook->>Standata: Retrieve workflow template
Standata-->>Notebook: Workflow definition
Notebook->>Notebook: Configure model & params
Notebook->>API: Create workflow
API-->>Notebook: Workflow ID
Notebook->>API: Get compute resources
API-->>Notebook: Available clusters
Notebook->>API: Create job
API->>QE: Submit job
QE-->>API: Job status
loop Poll for completion
Notebook->>API: Check job status
API-->>Notebook: Status updates
end
Notebook->>API: Retrieve results
API-->>Notebook: Relaxed structure & data
Notebook->>Notebook: Visualize results
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Ruff (0.15.5)other/materials_designer/workflows/Introduction.ipynbUnexpected end of JSON input other/materials_designer/workflows/relaxation.ipynbUnexpected end of JSON input 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 |
VsevolodX
changed the title
| @@ -0,0 +1,587 @@ | |||
| { | |||
Member
There was a problem hiding this comment.
| @@ -0,0 +1,587 @@ | |||
| { | |||
Member
There was a problem hiding this comment.
timurbazhirov
approved these changes
Mar 13, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@other/materials_designer/workflows/relaxation.ipynb`:
- Around line 475-478: The code assumes clusters is non-empty and that filtering
by CLUSTER_NAME returns a match; update the selection logic around CLUSTER_NAME
and cluster to guard these cases: first check if clusters is empty and raise or
log a clear error (or return early) if so, then when CLUSTER_NAME is set attempt
to find a match with next(...) into cluster and if that returns None fall back
to a safe default (e.g., clusters[0]) or raise a descriptive error; ensure any
downstream use of cluster (the variable named cluster) only happens after these
checks so cluster is never None.
- Around line 225-227: The code assumes projects[0] exists when fetching the
default project for ACCOUNT_ID; safeguard the access by checking that the
projects list returned from client.projects.list(...) is non-empty before using
projects[0] and assigning project_id; if empty, handle gracefully (e.g.,
log/raise a clear error mentioning ACCOUNT_ID and that no default project was
found, or create/select a project) so the subsequent print and indexing code
that uses project_id does not raise IndexError.
- Around line 582-586: Check that final_structure (the result of
client.properties.get_for_job with
PropertyName.non_scalar.final_structure.value) is non-empty before indexing [0];
if it's empty, raise or log a clear error/handle the incomplete job path instead
of dereferencing index 0. Specifically, guard the access used to compute
relaxed_material_id and the subsequent
Material.create(client.materials.get(...)) call by validating final_structure
and providing a descriptive error or fallback behavior when no final structure
is present.
- Around line 512-523: create_job is returning a List[dict] (per utils/api.py)
but the code treats job_response as a single dict and passes it to
dict_to_namespace then reads job._id; change the call-site to handle the list:
select the intended job dict from the returned list (e.g., first element) before
calling dict_to_namespace, or adjust create_job usage to unpack the single dict;
update variables job_response -> job_list (or similar) and then set job_dict =
job_list[0] (or the correct index) and call dict_to_namespace(job_dict) to
obtain job_id from job._id.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c666cad8-84fb-49fc-a65c-7492bcf004ae
📒 Files selected for processing (2)
other/materials_designer/workflows/Introduction.ipynbother/materials_designer/workflows/relaxation.ipynb
Comment on lines
+225
to
+227
| "projects = client.projects.list({\"isDefault\": True, \"owner._id\": ACCOUNT_ID})\n", | ||
| "project_id = projects[0][\"_id\"]\n", | ||
| "print(f\"\\u2705 Using project: {projects[0]['name']} ({project_id})\")" |
There was a problem hiding this comment.
Handle missing default project before indexing.
Line 226 assumes projects[0] always exists. If no default project is returned for the selected account, the notebook crashes with IndexError.
Suggested fix
projects = client.projects.list({"isDefault": True, "owner._id": ACCOUNT_ID})
-project_id = projects[0]["_id"]
-print(f"\u2705 Using project: {projects[0]['name']} ({project_id})")
+if not projects:
+ raise RuntimeError(f"No default project found for account {ACCOUNT_ID}")
+project_id = projects[0]["_id"]
+print(f"\u2705 Using project: {projects[0]['name']} ({project_id})")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@other/materials_designer/workflows/relaxation.ipynb` around lines 225 - 227,
The code assumes projects[0] exists when fetching the default project for
ACCOUNT_ID; safeguard the access by checking that the projects list returned
from client.projects.list(...) is non-empty before using projects[0] and
assigning project_id; if empty, handle gracefully (e.g., log/raise a clear error
mentioning ACCOUNT_ID and that no default project was found, or create/select a
project) so the subsequent print and indexing code that uses project_id does not
raise IndexError.
Comment on lines
+475
to
+478
| "if CLUSTER_NAME:\n", | ||
| " cluster = next((c for c in clusters if CLUSTER_NAME in c[\"hostname\"]), None)\n", | ||
| "else:\n", | ||
| " cluster = clusters[0]\n", |
There was a problem hiding this comment.
Guard against empty clusters and unmatched CLUSTER_NAME.
Line 478 assumes at least one cluster exists, and Line 481 can receive cluster=None when the name filter misses. Both cases lead to runtime failure later.
Suggested fix
if CLUSTER_NAME:
cluster = next((c for c in clusters if CLUSTER_NAME in c["hostname"]), None)
else:
- cluster = clusters[0]
+ cluster = clusters[0] if clusters else None
+
+if cluster is None:
+ available = [c["hostname"] for c in clusters]
+ raise ValueError(
+ f"Cluster not found. CLUSTER_NAME={CLUSTER_NAME!r}, available={available}"
+ )
compute = Compute(Also applies to: 480-485
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@other/materials_designer/workflows/relaxation.ipynb` around lines 475 - 478,
The code assumes clusters is non-empty and that filtering by CLUSTER_NAME
returns a match; update the selection logic around CLUSTER_NAME and cluster to
guard these cases: first check if clusters is empty and raise or log a clear
error (or return early) if so, then when CLUSTER_NAME is set attempt to find a
match with next(...) into cluster and if that returns None fall back to a safe
default (e.g., clusters[0]) or raise a descriptive error; ensure any downstream
use of cluster (the variable named cluster) only happens after these checks so
cluster is never None.
Comment on lines
+512
to
+523
| "job_response = create_job(\n", | ||
| " api_client=client,\n", | ||
| " materials=[saved_material],\n", | ||
| " workflow=workflow,\n", | ||
| " project_id=project_id,\n", | ||
| " owner_id=ACCOUNT_ID,\n", | ||
| " prefix=job_name,\n", | ||
| " compute=compute.to_dict(),\n", | ||
| ")\n", | ||
| "\n", | ||
| "job = dict_to_namespace(job_response)\n", | ||
| "job_id = job._id\n", |
There was a problem hiding this comment.
create_job() response is treated as a dict, but helper returns a list.
At Line 522+, the code assumes a single dict (job_response). The helper signature in utils/api.py returns List[dict], so this path can fail when reading _id.
Suggested fix
-job_response = create_job(
+job_responses = create_job(
api_client=client,
materials=[saved_material],
workflow=workflow,
project_id=project_id,
owner_id=ACCOUNT_ID,
prefix=job_name,
compute=compute.to_dict(),
)
-job = dict_to_namespace(job_response)
+if not job_responses:
+ raise RuntimeError("No jobs were created")
+job_response = job_responses[0]
+job = dict_to_namespace(job_response)
job_id = job._id
print("\u2705 Job created successfully!")
print(f"Job ID: {job_id}")
display_JSON(job_response)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@other/materials_designer/workflows/relaxation.ipynb` around lines 512 - 523,
create_job is returning a List[dict] (per utils/api.py) but the code treats
job_response as a single dict and passes it to dict_to_namespace then reads
job._id; change the call-site to handle the list: select the intended job dict
from the returned list (e.g., first element) before calling dict_to_namespace,
or adjust create_job usage to unpack the single dict; update variables
job_response -> job_list (or similar) and then set job_dict = job_list[0] (or
the correct index) and call dict_to_namespace(job_dict) to obtain job_id from
job._id.
Comment on lines
+582
to
+586
| "final_structure = client.properties.get_for_job(\n", | ||
| " job_id, property_name=PropertyName.non_scalar.final_structure.value\n", | ||
| ")\n", | ||
| "relaxed_material_id = final_structure[0][\"materialId\"]\n", | ||
| "relaxed_material = Material.create(client.materials.get(relaxed_material_id))\n", |
There was a problem hiding this comment.
Check final_structure before dereferencing index 0.
Line 585 assumes a final-structure property is always present. Failed/incomplete jobs can return an empty list and crash here.
Suggested fix
final_structure = client.properties.get_for_job(
job_id, property_name=PropertyName.non_scalar.final_structure.value
)
-relaxed_material_id = final_structure[0]["materialId"]
+if not final_structure:
+ raise RuntimeError(f"No final structure found for job {job_id}")
+relaxed_material_id = final_structure[0]["materialId"]
relaxed_material = Material.create(client.materials.get(relaxed_material_id))📝 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.
Suggested change
| "final_structure = client.properties.get_for_job(\n", | |
| " job_id, property_name=PropertyName.non_scalar.final_structure.value\n", | |
| ")\n", | |
| "relaxed_material_id = final_structure[0][\"materialId\"]\n", | |
| "relaxed_material = Material.create(client.materials.get(relaxed_material_id))\n", | |
| final_structure = client.properties.get_for_job( | |
| job_id, property_name=PropertyName.non_scalar.final_structure.value | |
| ) | |
| if not final_structure: | |
| raise RuntimeError(f"No final structure found for job {job_id}") | |
| relaxed_material_id = final_structure[0]["materialId"] | |
| relaxed_material = Material.create(client.materials.get(relaxed_material_id)) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@other/materials_designer/workflows/relaxation.ipynb` around lines 582 - 586,
Check that final_structure (the result of client.properties.get_for_job with
PropertyName.non_scalar.final_structure.value) is non-empty before indexing [0];
if it's empty, raise or log a clear error/handle the incomplete job path instead
of dereferencing index 0. Specifically, guard the access used to compute
relaxed_material_id and the subsequent
Material.create(client.materials.get(...)) call by validating final_structure
and providing a descriptive error or fallback behavior when no final structure
is present.
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
2 participants
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.
Summary by CodeRabbit
Documentation