Skip to content

Conversation

@daquinteroflex
Copy link
Collaborator

@daquinteroflex daquinteroflex commented Oct 15, 2025

image image image

Doesn't solve the batch issue, but updates us to the new required lambdas. Still need to add tests but will do once things are more stable.

@daquinteroflex daquinteroflex force-pushed the FXC-3690-update-cm-pre-processing-lambda-to-async-flow branch 3 times, most recently from d17c858 to 6e69702 Compare October 17, 2025 13:26
@daquinteroflex daquinteroflex changed the base branch from develop to FXC-3689-per-simulation-downloads-during-batch-runs October 17, 2025 13:26
@daquinteroflex daquinteroflex marked this pull request as ready for review October 17, 2025 13:33
@daquinteroflex daquinteroflex added rc3 3rd pre-release 2.10 labels Oct 17, 2025
Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 files reviewed, 4 comments

Edit Code Review Agent Settings | Greptile

@daquinteroflex daquinteroflex force-pushed the FXC-3690-update-cm-pre-processing-lambda-to-async-flow branch 2 times, most recently from 886c148 to 9eba73e Compare October 17, 2025 14:02
Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

update

@marcorudolphflex marcorudolphflex force-pushed the FXC-3689-per-simulation-downloads-during-batch-runs branch from c2e34b5 to 5ec4203 Compare October 17, 2025 14:18
@daquinteroflex daquinteroflex force-pushed the FXC-3690-update-cm-pre-processing-lambda-to-async-flow branch from 9eba73e to 64086af Compare October 17, 2025 14:20
@daquinteroflex
Copy link
Collaborator Author

@greptile re evaluate confidence score again edit your previous comment

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

update

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at this PR, I can see the changes implement an asynchronous preprocessing lambda for RF (radio frequency) component modeler tasks. Let me re-evaluate my confidence assessment:

Updated Confidence Assessment: 8.5/10 ✅

What increases my confidence:

  1. Well-structured refactoring: The code moves from synchronous to asynchronous processing with proper separation of concerns - extracting logic into dedicated functions like _upload_component_modeler_subtasks and _batch_detail_error

  2. Comprehensive error handling: The new _batch_detail_error function provides detailed error parsing, especially for validation failures, with proper JSON parsing and fallback handling

  3. Progress monitoring: The implementation includes proper progress tracking for both split and validation phases, with appropriate state management

  4. Consistent API patterns: The new AsyncJobDetail model follows existing patterns and integrates well with the current task management system

  5. Backward compatibility: The changes maintain the same external interface while improving internal implementation

Minor concerns that slightly reduce confidence:

  1. Complex nested logic: The _upload_component_modeler_subtasks function handles multiple phases (split + validation) which could potentially be further decomposed

  2. State management complexity: The function handles both verbose and non-verbose modes with different code paths, though this is reasonable

  3. Error propagation: The function returns errors rather than raising them in some cases, which should be consistent with the existing error handling patterns

Overall, this is a solid implementation that improves the user experience by providing better progress feedback and more detailed error reporting for component modeler operations. The async processing should also improve system responsiveness.

@daquinteroflex daquinteroflex force-pushed the FXC-3690-update-cm-pre-processing-lambda-to-async-flow branch from 64086af to d55c38e Compare October 17, 2025 14:46
@daquinteroflex daquinteroflex changed the base branch from FXC-3689-per-simulation-downloads-during-batch-runs to develop October 17, 2025 15:21
@github-actions
Copy link
Contributor

github-actions bot commented Oct 17, 2025

Diff Coverage

Diff: origin/develop...HEAD, staged and unstaged changes

  • tidy3d/web/api/webapi.py (3.0%): Missing lines 112-117,119-120,122,124,129-137,139-140,142-143,147,151,155,188-190,192-193,200-201,203-205,207-208,216-221,223,227-228,233-234,236-238,240-242,245-246,248-252,256-259,262-268,272-277,280-282,285-293,295,310-311,596,741,1593
  • tidy3d/web/core/task_info.py (100%)

Summary

  • Total: 111 lines
  • Missing: 97 lines
  • Coverage: 12%

tidy3d/web/api/webapi.py

  108 
  109     Returns:
  110         An instance of `WebError` if the batch failed, otherwise `None`.
  111     """
! 112     try:
! 113         batch_detail = BatchTask(batch_id=resource_id).detail(batch_type="RF_SWEEP")
! 114         status = batch_detail.totalStatus.value
! 115     except Exception as e:
! 116         log.error(f"Could not retrieve batch details for '{resource_id}': {e}")
! 117         return WebError(f"Failed to retrieve status for batch '{resource_id}'.")
  118 
! 119     if status not in ERROR_STATES:
! 120         return None
  121 
! 122     log.error(f"The ComponentModeler batch '{resource_id}' has failed with status: {status}")
  123 
! 124     if (
  125         status == "validate_fail"
  126         and hasattr(batch_detail, "validateErrors")
  127         and batch_detail.validateErrors
  128     ):
! 129         error_details = []
! 130         for key, error_str in batch_detail.validateErrors.items():
! 131             try:
! 132                 error_dict = json.loads(error_str)
! 133                 validation_error = error_dict.get("validation_error", "Unknown validation error.")
! 134                 msg = f"- Subtask '{key}' failed: {validation_error}"
! 135                 log.error(msg)
! 136                 error_details.append(msg)
! 137             except (json.JSONDecodeError, TypeError):
  138                 # Handle cases where the error string isn't valid JSON
! 139                 log.error(f"Could not parse validation error for subtask '{key}'.")
! 140                 error_details.append(f"- Subtask '{key}': Could not parse error details.")
  141 
! 142         details_string = "\n".join(error_details)
! 143         full_error_msg = (
  144             "One or more subtasks failed validation. Please fix the component modeler configuration.\n"
  145             f"Details:\n{details_string}"
  146         )
! 147         return WebError(full_error_msg)
  148 
  149     # Handle all other generic error states
  150     else:
! 151         error_msg = (
  152             f"Batch '{resource_id}' failed with status '{status}'. Check server "
  153             "logs for details or contact customer support."
  154         )
! 155         return WebError(error_msg)
  156 
  157 
  158 def _upload_component_modeler_subtasks(
  159     resource_id: str, verbose: bool = True, solver_version: Optional[str] = None

  184         RuntimeError: If the initial asynchronous split job fails.
  185         WebError: If the subsequent batch validation fails, ends in an
  186             unexpected state, or if a 'validate_fail' status is encountered.
  187     """
! 188     console = get_logging_console() if verbose else None
! 189     final_error = None
! 190     batch_type = "RF_SWEEP"
  191 
! 192     split_path = "tidy3d/async-biz/component-modeler-split"
! 193     payload = {
  194         "batchType": batch_type,
  195         "batchId": resource_id,
  196         "fileName": "modeler.hdf5.gz",
  197         "protocolVersion": _get_protocol_version(),

  196         "fileName": "modeler.hdf5.gz",
  197         "protocolVersion": _get_protocol_version(),
  198     }
  199 
! 200     if verbose:
! 201         console.log("Starting Modeler and Subtasks Validation...")
  202 
! 203     initial_resp = http.post(split_path, payload)
! 204     split_job_detail = AsyncJobDetail(**initial_resp)
! 205     monitor_split_path = f"{split_path}?asyncId={split_job_detail.asyncId}"
  206 
! 207     if verbose:
! 208         progress_bar = Progress(
  209             TextColumn("[progress.description]{task.description}"),
  210             BarColumn(),
  211             TaskProgressColumn(),
  212             TimeElapsedColumn(),

  212             TimeElapsedColumn(),
  213             console=console,
  214         )
  215 
! 216         with progress_bar as progress:
! 217             description = "Upload Subtasks"
! 218             pbar = progress.add_task(description, completed=split_job_detail.progress, total=100)
! 219             while True:
! 220                 split_job_raw_result = http.get(monitor_split_path)
! 221                 split_job_detail = AsyncJobDetail(**split_job_raw_result)
  222 
! 223                 progress.update(
  224                     pbar, completed=split_job_detail.progress, description=f"[blue]{description}"
  225                 )
  226 
! 227                 if split_job_detail.status in END_STATES:
! 228                     progress.update(
  229                         pbar,
  230                         completed=split_job_detail.progress,
  231                         description=f"[green]{description}",
  232                     )
! 233                     break
! 234                 time.sleep(RUN_REFRESH_TIME)
  235 
! 236             if split_job_detail.status in ERROR_STATES:
! 237                 msg = split_job_detail.message or "An unknown error occurred."
! 238                 final_error = WebError(f"Component modeler split job failed: {msg}")
  239 
! 240             if not final_error:
! 241                 description = "Validating"
! 242                 pbar = progress.add_task(
  243                     completed=10, total=100, description=f"[blue]{description}"
  244                 )
! 245                 batch = BatchTask(resource_id)
! 246                 batch.check(solver_version=solver_version, batch_type=batch_type)
  247 
! 248                 while True:
! 249                     batch_detail = batch.detail(batch_type=batch_type)
! 250                     status = batch_detail.totalStatus
! 251                     progress_percent = STATE_PROGRESS_PERCENTAGE.get(status, 0)
! 252                     progress.update(
  253                         pbar, completed=progress_percent, description=f"[blue]{description}"
  254                     )
  255 
! 256                     if status in POST_VALIDATE_STATES:
! 257                         progress.update(pbar, completed=100, description=f"[green]{description}")
! 258                         task_mapping = json.loads(split_job_detail.result)
! 259                         console.log(
  260                             f"Uploaded Subtasks: \n{_task_dict_to_url_bullet_list(task_mapping)}"
  261                         )
! 262                         progress.refresh()
! 263                         break
! 264                     elif status in ERROR_STATES:
! 265                         progress.update(pbar, completed=0, description=f"[red]{description}")
! 266                         progress.refresh()
! 267                         break
! 268                     time.sleep(RUN_REFRESH_TIME)
  269 
  270     else:
  271         # Non-verbose mode: Poll for split job completion.
! 272         while True:
! 273             split_job_raw_result = http.get(monitor_split_path)
! 274             split_job_detail = AsyncJobDetail(**split_job_raw_result)
! 275             if split_job_detail.status in END_STATES:
! 276                 break
! 277             time.sleep(RUN_REFRESH_TIME)
  278 
  279         # Check for split job failure.
! 280         if split_job_detail.status in ERROR_STATES:
! 281             msg = split_job_detail.message or "An unknown error occurred."
! 282             final_error = WebError(f"Component modeler split job failed: {msg}")
  283 
  284         # If split succeeded, poll for validation completion.
! 285         if not final_error:
! 286             batch = BatchTask(resource_id)
! 287             batch.check(solver_version=solver_version, batch_type=batch_type)
! 288             while True:
! 289                 batch_detail = batch.detail(batch_type=batch_type)
! 290                 status = batch_detail.totalStatus
! 291                 if status in POST_VALIDATE_STATES or status in END_STATES:
! 292                     break
! 293                 time.sleep(RUN_REFRESH_TIME)
  294 
! 295     return _batch_detail_error(resource_id=resource_id)
  296 
  297 
  298 def _task_dict_to_url_bullet_list(data_dict: dict) -> str:
  299     """

  306       A string with each key-url/value pair as a bullet point.
  307     """
  308     # Use a list comprehension to format each key-value pair
  309     # and then join them together with newline characters.
! 310     if data_dict is None:
! 311         raise WebError("Error in subtask dictionary data.")
  312     return "\n".join([f"- {key}: '{value}'" for key, value in data_dict.items()])
  313 
  314 
  315 @wait_for_connection

  592         remote_sim_file=remote_sim_file,
  593     )
  594 
  595     if task_type == "RF":
! 596         _upload_component_modeler_subtasks(resource_id=resource_id, verbose=verbose)
  597 
  598     estimate_cost(task_id=resource_id, solver_version=solver_version, verbose=verbose)
  599 
  600     task.validate_post_upload(parent_tasks=parent_tasks)

  737         batch.submit(
  738             solver_version=solver_version, batch_type="RF_SWEEP", worker_group=worker_group
  739         )
  740         if verbose:
! 741             console.log(f"Component Modeler '{task_id}' validated. Solving...")
  742         return
  743 
  744     if priority is not None and (priority < 1 or priority > 10):
  745         raise ValueError("Priority must be between '1' and '10' if specified.")

  1589                 )
  1590             return est_flex_unit
  1591 
  1592         elif status in ERROR_STATES:
! 1593             return _batch_detail_error(resource_id=task_id)
  1594 
  1595         raise WebError("Could not get estimated cost!")
  1596 
  1597     else:

@daquinteroflex daquinteroflex added this pull request to the merge queue Oct 17, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 17, 2025
@daquinteroflex daquinteroflex force-pushed the FXC-3690-update-cm-pre-processing-lambda-to-async-flow branch from d55c38e to 25b304e Compare October 17, 2025 16:50
@daquinteroflex daquinteroflex added this pull request to the merge queue Oct 17, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 17, 2025
@daquinteroflex daquinteroflex force-pushed the FXC-3690-update-cm-pre-processing-lambda-to-async-flow branch from 25b304e to ced9cc7 Compare October 19, 2025 19:40
@daquinteroflex daquinteroflex added this pull request to the merge queue Oct 19, 2025
Merged via the queue into develop with commit 1262810 Oct 19, 2025
25 checks passed
@daquinteroflex daquinteroflex deleted the FXC-3690-update-cm-pre-processing-lambda-to-async-flow branch October 19, 2025 20:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2.10 rc3 3rd pre-release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants