FXC-3294-add-opt-in-local-cache-for-simulation-results-hashed-by-simulation-runtime-context #2871
Conversation
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
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
tidy3d/web/cache.py
Outdated
|
|
||
| return entry | ||
| except Exception as e: | ||
| log.error("Failed to fetch cache results." + str(e)) |
There was a problem hiding this comment.
syntax: Missing space before concatenation - should be "Failed to fetch cache results: " + str(e)
Suggested change
| log.error("Failed to fetch cache results." + str(e)) | |
| log.error("Failed to fetch cache results: " + str(e)) |
Prompt To Fix With AI
This is a comment left during a code review.
Path: tidy3d/web/cache.py
Line: 529:529
Comment:
**syntax:** Missing space before concatenation - should be `"Failed to fetch cache results: " + str(e)`
```suggestion
log.error("Failed to fetch cache results: " + str(e))
```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+573
to
+574
| except Exception: | ||
| log.error("Could not store cache entry.") |
There was a problem hiding this comment.
style: Generic exception handling without logging the actual error details - consider logging the exception for debugging
Prompt To Fix With AI
This is a comment left during a code review.
Path: tidy3d/web/cache.py
Line: 573:574
Comment:
**style:** Generic exception handling without logging the actual error details - consider logging the exception for debugging
How can I resolve this? If you propose a fix, please make it concise.
tidy3d/web/cache.py
Outdated
| entry = self._fetch(cache_key) | ||
| if not entry: | ||
| return None | ||
| # self._store(key=cache_key, task_id=task_id, source_path=path, metadata={}) |
There was a problem hiding this comment.
style: Remove commented-out code before finalizing the PR
Suggested change
| # self._store(key=cache_key, task_id=task_id, source_path=path, metadata={}) | |
| if not entry: | |
| return None |
Context Used: Rule from dashboard - Remove temporary debugging code (print() calls), commented-out code, and other workarounds before fi... (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: tidy3d/web/cache.py
Line: 521:521
Comment:
**style:** Remove commented-out code before finalizing the PR
```suggestion
if not entry:
return None
```
**Context Used:** Rule from `dashboard` - Remove temporary debugging code (print() calls), commented-out code, and other workarounds before fi... ([source](https://app.greptile.com/review/custom-context?memory=f6a669d8-0060-4f11-9cac-10ac7ee749ea))
How can I resolve this? If you propose a fix, please make it concise.
tidy3d/web/api/autograd/autograd.py
Outdated
| Whether to reduce structures in the simulation to the simulation domain only. Note: currently only implemented for the mode solver. | ||
| pay_type: typing.Union[PayType, str] = PayType.AUTO | ||
| Specify the payment method. | ||
| Whether to reduce structures in the simulation to the simulation domain only. Note: currently only implemented for the mode solver. |
There was a problem hiding this comment.
syntax: Duplicate docstring line detected - 'Whether to reduce structures...' appears twice
Suggested change
| Whether to reduce structures in the simulation to the simulation domain only. Note: currently only implemented for the mode solver. | |
| Specify the payment method. |
Prompt To Fix With AI
This is a comment left during a code review.
Path: tidy3d/web/api/autograd/autograd.py
Line: 328:328
Comment:
**syntax:** Duplicate docstring line detected - 'Whether to reduce structures...' appears twice
```suggestion
Specify the payment method.
```
How can I resolve this? If you propose a fix, please make it concise.
tidy3d/web/api/webapi.py
Outdated
Comment on lines
127
to
134
| def _get_simulation_data_from_cache_entry(entry: CacheEntry, path: str) -> bool: | ||
| if entry is not None: | ||
| try: | ||
| entry.materialize(Path(path)) | ||
| return True | ||
| except Exception: | ||
| return False | ||
| return False |
There was a problem hiding this comment.
style: The broad exception handling here could mask important errors. Consider logging the specific exception or handling specific exception types (FileNotFoundError, PermissionError, etc.)
Prompt To Fix With AI
This is a comment left during a code review.
Path: tidy3d/web/api/webapi.py
Line: 127:134
Comment:
**style:** The broad exception handling here could mask important errors. Consider logging the specific exception or handling specific exception types (FileNotFoundError, PermissionError, etc.)
How can I resolve this? If you propose a fix, please make it concise.| Union[:class:`.SimulationData`, :class:`.HeatSimulationData`, :class:`.EMESimulationData`] | ||
| Object containing simulation data. | ||
| """ | ||
| assert from_cache or task_id, "Either task_id or from_cache must be provided." |
There was a problem hiding this comment.
style: This assertion could provide a more descriptive error message to help users understand when task_id is required vs when from_cache should be used
Prompt To Fix With AI
This is a comment left during a code review.
Path: tidy3d/web/api/webapi.py
Line: 1076:1076
Comment:
**style:** This assertion could provide a more descriptive error message to help users understand when task_id is required vs when from_cache should be used
How can I resolve this? If you propose a fix, please make it concise.| cache.store_result(_FakeStubData(sim2), MOCK_TASK_ID, str(file2), "FDTD") | ||
|
|
||
| entries = cache.list() | ||
| print("len(entries)", len(entries)) |
There was a problem hiding this comment.
style: Remove debugging print statement
Suggested change
| print("len(entries)", len(entries)) | |
| entries = cache.list() | |
| assert len(cache) == 1 | |
| assert entries[0]["simulation_hash"] == sim2._hash_self() |
Context Used: Rule from dashboard - Remove temporary debugging code (print() calls), commented-out code, and other workarounds before fi... (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: tests/test_web/test_simulation_cache.py
Line: 268:268
Comment:
**style:** Remove debugging print statement
```suggestion
entries = cache.list()
assert len(cache) == 1
assert entries[0]["simulation_hash"] == sim2._hash_self()
```
**Context Used:** Rule from `dashboard` - Remove temporary debugging code (print() calls), commented-out code, and other workarounds before fi... ([source](https://app.greptile.com/review/custom-context?memory=f6a669d8-0060-4f11-9cac-10ac7ee749ea))
How can I resolve this? If you propose a fix, please make it concise.
tidy3d/web/api/container.py
Outdated
| shutil.move(self.data_cache_path, path) | ||
| self._cache_file_moved = True | ||
| else: | ||
| raise FileNotFoundError(f"Cached file does not longer exist in {path}.") |
There was a problem hiding this comment.
syntax: Error message incorrectly references 'path' parameter instead of 'self.data_cache_path'
Suggested change
| raise FileNotFoundError(f"Cached file does not longer exist in {path}.") | |
| raise FileNotFoundError(f"Cached file does not longer exist in {self.data_cache_path}.") |
Prompt To Fix With AI
This is a comment left during a code review.
Path: tidy3d/web/api/container.py
Line: 451:451
Comment:
**syntax:** Error message incorrectly references 'path' parameter instead of 'self.data_cache_path'
```suggestion
raise FileNotFoundError(f"Cached file does not longer exist in {self.data_cache_path}.")
```
How can I resolve this? If you propose a fix, please make it concise.
tidy3d/web/api/container.py
Outdated
Comment on lines
286
to
288
| use_cache: Optional[bool] = None | ||
| Override cache usage behaviour for this call. ``True`` forces cache usage when available, | ||
| ``False`` bypasses it, and ``None`` defers to configuration and environment settings. |
There was a problem hiding this comment.
syntax: Docstring mentions 'use_cache' parameter but method signature doesn't include it
Suggested change
| use_cache: Optional[bool] = None | |
| Override cache usage behaviour for this call. ``True`` forces cache usage when available, | |
| ``False`` bypasses it, and ``None`` defers to configuration and environment settings. | |
| Returns | |
| ------- | |
| :class:`WorkflowDataType` | |
| Object containing simulation results. |
Context Used: Rule from dashboard - Keep docstrings and comments synchronized with code changes to ensure they are always accurate and n... (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: tidy3d/web/api/container.py
Line: 286:288
Comment:
**syntax:** Docstring mentions 'use_cache' parameter but method signature doesn't include it
```suggestion
Returns
-------
:class:`WorkflowDataType`
Object containing simulation results.
```
**Context Used:** Rule from `dashboard` - Keep docstrings and comments synchronized with code changes to ensure they are always accurate and n... ([source](https://app.greptile.com/review/custom-context?memory=8947a127-e303-4dbf-b326-493579e5f047))
How can I resolve this? If you propose a fix, please make it concise.
tidy3d/web/api/container.py
Outdated
Comment on lines
430
to
432
| use_cache: Optional[bool] = None | ||
| Override cache usage behaviour for this call. ``True`` forces cache usage when available, | ||
| ``False`` bypasses it, and ``None`` defers to configuration and environment settings. |
There was a problem hiding this comment.
syntax: Docstring mentions 'use_cache' parameter but method signature doesn't include it
Suggested change
| use_cache: Optional[bool] = None | |
| Override cache usage behaviour for this call. ``True`` forces cache usage when available, | |
| ``False`` bypasses it, and ``None`` defers to configuration and environment settings. | |
| Note | |
| ---- | |
| To load the data after download, use :meth:`Job.load`. |
Context Used: Rule from dashboard - Keep docstrings and comments synchronized with code changes to ensure they are always accurate and n... (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: tidy3d/web/api/container.py
Line: 430:432
Comment:
**syntax:** Docstring mentions 'use_cache' parameter but method signature doesn't include it
```suggestion
Note
----
To load the data after download, use :meth:`Job.load`.
```
**Context Used:** Rule from `dashboard` - Keep docstrings and comments synchronized with code changes to ensure they are always accurate and n... ([source](https://app.greptile.com/review/custom-context?memory=8947a127-e303-4dbf-b326-493579e5f047))
How can I resolve this? If you propose a fix, please make it concise.
Diff CoverageDiff: origin/develop...HEAD, staged and unstaged changes
Summary
tidy3d/web/api/container.pyLines 258-269 258 simulation_cache = resolve_simulation_cache(self.use_cache)
259 if simulation_cache is not None:
260 sim_for_cache = self.simulation
261 if isinstance(self.simulation, (ModeSolver, ModeSimulation)):
! 262 sim_for_cache = get_reduced_simulation(self.simulation, self.reduce_simulation)
263 entry = simulation_cache.try_fetch(simulation=sim_for_cache)
264 return entry
! 265 return None
266
267 def run(
268 self,
269 path: str = DEFAULT_DATA_PATH,Lines 290-298 290 self.upload()
291 if priority is None:
292 self.start()
293 else:
! 294 self.start(priority=priority)
295 self.monitor()
296 data = self.load(path=path)
297
298 return dataLines 363-371 363 @property
364 def status(self):
365 """Return current status of :class:`Job`."""
366 if self.load_if_cached:
! 367 return "success"
368 return self.get_info().status
369
370 def start(self, priority: Optional[int] = None) -> None:
371 """Start running a :class:`Job`.Lines 408-416 408 To load the output of completed simulation into :class:`.SimulationData` objects,
409 call :meth:`Job.load`.
410 """
411 if self.load_if_cached:
! 412 return
413 web.monitor(self.task_id, verbose=self.verbose)
414
415 def download(self, path: str = DEFAULT_DATA_PATH) -> None:
416 """Download results of simulation.Lines 431-444 431 web.download(task_id=self.task_id, path=path, verbose=self.verbose)
432
433 def move_cache_file(self, path: str) -> None:
434 if self._cache_file_moved:
! 435 return
436 if os.path.exists(self.data_cache_path):
437 shutil.move(self.data_cache_path, path)
438 self._cache_file_moved = True
439 else:
! 440 raise FileNotFoundError(f"Cached file does not longer exist in {self.data_cache_path}.")
441
442 def load(self, path: str = DEFAULT_DATA_PATH) -> WorkflowDataType:
443 """Download job results and load them into a data object.Lines 789-797 789 self.to_file(self._batch_path(path_dir=path_dir))
790 if priority is None:
791 self.start()
792 else:
! 793 self.start(priority=priority)
794 self.monitor()
795 return self.load(path_dir=path_dir)
796
797 @cached_propertytidy3d/web/api/webapi.pyLines 128-137 128 if entry is not None:
129 try:
130 entry.materialize(Path(path))
131 return True
! 132 except Exception:
! 133 return False
134 return False
135
136
137 @wait_for_connectionLines 243-251 243 loaded_from_cache = False
244 if simulation_cache is not None:
245 sim_for_cache = simulation
246 if isinstance(simulation, (ModeSolver, ModeSimulation)):
! 247 sim_for_cache = get_reduced_simulation(simulation, reduce_simulation)
248 entry = simulation_cache.try_fetch(simulation=sim_for_cache)
249 loaded_from_cache = _get_simulation_data_from_cache_entry(entry, path)
250
251 if not loaded_from_cache:Lines 1085-1093 1085 path = os.path.join(base_dir, "cm_data.hdf5")
1086
1087 if from_cache:
1088 if not os.path.exists(path):
! 1089 raise FileNotFoundError("Cached file not found.")
1090 elif not os.path.exists(path) or replace_existing:
1091 download(task_id=task_id, path=path, verbose=verbose, progress_callback=progress_callback)
1092
1093 if verbose:tidy3d/web/cache.pyLines 54-64 54 return None
55 normalized = value.strip().lower()
56 if normalized in {"1", "true", "yes", "on"}:
57 return True
! 58 if normalized in {"0", "false", "no", "off"}:
! 59 return False
! 60 return None
61
62
63 def _coerce_float(value: str) -> Optional[float]:
64 if value is None:Lines 64-73 64 if value is None:
65 return None
66 try:
67 return float(value)
! 68 except (TypeError, ValueError):
! 69 return None
70
71
72 def _coerce_int(value: str) -> Optional[int]:
73 if value is None:Lines 73-82 73 if value is None:
74 return None
75 try:
76 return int(value)
! 77 except (TypeError, ValueError):
! 78 return None
79
80
81 def _load_env_overrides() -> dict[str, Any]:
82 overrides: dict[str, Any] = {}Lines 165-177 165 def _apply_overrides(
166 cfg: SimulationCacheConfig, overrides: dict[str, Any]
167 ) -> SimulationCacheConfig:
168 """Apply dict-based overrides (enabled/directory/max_size_gb/max_entries)."""
! 169 if not overrides:
! 170 return cfg
171 # Filter to fields that exist on the dataclass and are not None
! 172 allowed = {k: v for k, v in overrides.items() if v is not None and hasattr(cfg, k)}
! 173 return replace(cfg, **allowed) if allowed else cfg
174
175
176 def resolve_simulation_cache(use_cache: Optional[bool] = None) -> Optional[SimulationCache]:
177 """Lines 198-208 198 return None
199
200 try:
201 return get_cache()
! 202 except Exception as err:
! 203 log.debug("Simulation cache unavailable: %s", err)
! 204 return None
205
206
207 @dataclass
208 class CacheEntry:Lines 228-243 228 return self.path.exists() and self.artifact_path.exists() and self.metadata_path.exists()
229
230 def verify(self) -> bool:
231 if not self.exists():
! 232 return False
233 checksum = self.metadata.get("checksum")
234 if not checksum:
! 235 return False
236 try:
237 actual_checksum, file_size = _copy_and_hash(self.artifact_path, None)
! 238 except FileNotFoundError:
! 239 return False
240 if checksum != actual_checksum:
241 log.warning(
242 "Simulation cache checksum mismatch for key '%s'. Removing stale entry.", self.key
243 )Lines 242-251 242 "Simulation cache checksum mismatch for key '%s'. Removing stale entry.", self.key
243 )
244 return False
245 if int(self.metadata.get("file_size", file_size)) != file_size:
! 246 self.metadata["file_size"] = file_size
! 247 _write_metadata(self.metadata_path, self.metadata)
248 return True
249
250 def materialize(self, target: Path) -> Path:
251 """Copy cached artifact to ``target`` and return the resulting path."""Lines 285-294 285 try:
286 shutil.rmtree(self._root)
287 if not hard:
288 self._root.mkdir(parents=True, exist_ok=True)
! 289 except (FileNotFoundError, OSError):
! 290 pass
291
292 def _fetch(self, key: str) -> Optional[CacheEntry]:
293 """Retrieve an entry by key, verifying checksum."""
294 with self._lock:Lines 302-320 302 return entry
303
304 def fetch_by_task(self, task_id: str) -> Optional[CacheEntry]:
305 """Retrieve an entry by task id."""
! 306 with self._lock:
! 307 for entry in self._iter_entries():
! 308 metadata = entry.metadata
! 309 task_ids = metadata.get("task_ids", [])
! 310 if task_id in task_ids and entry.exists():
! 311 if not entry.verify():
! 312 self._remove_entry(entry)
! 313 return None
! 314 self._touch(entry)
! 315 return entry
! 316 return None
317
318 def __len__(self) -> int:
319 """Return number of valid cache entries."""
320 with self._lock:Lines 342-350 342 Representation of the stored cache entry.
343 """
344 source_path = Path(source_path)
345 if not source_path.exists():
! 346 raise FileNotFoundError(f"Cannot cache missing artifact: {source_path}")
347 os.makedirs(self._root, exist_ok=True)
348 tmp_dir = Path(tempfile.mkdtemp(prefix=TMP_PREFIX, dir=self._root))
349 tmp_artifact = tmp_dir / CACHE_ARTIFACT_NAME
350 tmp_meta = tmp_dir / CACHE_METADATA_NAMELines 373-395 373 backup_dir: Optional[Path] = None
374
375 try:
376 if final_dir.exists():
! 377 backup_dir = final_dir.with_name(
378 f"{final_dir.name}.bak.{_timestamp_suffix()}"
379 )
! 380 os.replace(final_dir, backup_dir)
381 # move tmp_dir into place
382 os.replace(tmp_dir, final_dir)
! 383 except Exception:
384 # restore backup if needed
! 385 if backup_dir and backup_dir.exists():
! 386 os.replace(backup_dir, final_dir)
! 387 raise
388 else:
389 entry = CacheEntry(key=key, root=self._root, metadata=metadata)
390 if backup_dir and backup_dir.exists():
! 391 shutil.rmtree(backup_dir, ignore_errors=True)
392 log.debug("Stored simulation cache entry '%s' (%d bytes).", key, file_size)
393 return entry
394 finally:
395 try:Lines 393-409 393 return entry
394 finally:
395 try:
396 if tmp_dir.exists():
! 397 shutil.rmtree(tmp_dir, ignore_errors=True)
! 398 except FileNotFoundError:
! 399 pass
400
401 def invalidate(self, key: str) -> None:
! 402 with self._lock:
! 403 entry = self._load_entry(key)
! 404 if entry:
! 405 self._remove_entry(entry)
406
407 def _ensure_limits(self, incoming_size: int) -> None:
408 max_entries = max(self._config.max_entries, 0)
409 max_size_bytes = int(max(0.0, self._config.max_size_gb) * (1024**3))Lines 413-426 413 self._evict(entries, keep=max_entries - 1)
414 entries = list(self._iter_entries())
415
416 if not max_size_bytes:
! 417 return
418
419 existing_size = sum(int(e.metadata.get("file_size", 0)) for e in entries)
420 allowed_size = max(max_size_bytes - incoming_size, 0)
421 if existing_size > allowed_size:
! 422 self._evict_by_size(entries, existing_size, allowed_size)
423
424 def _evict(self, entries: Iterable[CacheEntry], keep: int) -> None:
425 sorted_entries = sorted(entries, key=lambda e: e.metadata.get("last_used", ""))
426 to_remove = sorted_entries[: max(0, len(sorted_entries) - keep)]Lines 429-451 429
430 def _evict_by_size(
431 self, entries: Iterable[CacheEntry], current_size: int, allowed_size: float
432 ) -> None:
! 433 if allowed_size < 0:
! 434 allowed_size = 0
! 435 sorted_entries = sorted(entries, key=lambda e: e.metadata.get("last_used", ""))
! 436 reclaimed = 0
! 437 for entry in sorted_entries:
! 438 if current_size - reclaimed <= allowed_size:
! 439 break
! 440 size = int(entry.metadata.get("file_size", 0))
! 441 self._remove_entry(entry)
! 442 reclaimed += size
! 443 log.info(f"Simulation cache evicted entry '{entry.key}' to reclaim {size} bytes.")
444
445 def _iter_entries(self) -> Iterable[CacheEntry]:
446 if not self._root.exists():
! 447 return []
448 entries: list[CacheEntry] = []
449 for child in self._root.iterdir():
450 if child.name.startswith(TMP_PREFIX) or child.name.startswith(TMP_BATCH_PREFIX):
451 continueLines 450-462 450 if child.name.startswith(TMP_PREFIX) or child.name.startswith(TMP_BATCH_PREFIX):
451 continue
452 meta_path = child / CACHE_METADATA_NAME
453 if not meta_path.exists():
! 454 continue
455 try:
456 metadata = json.loads(meta_path.read_text(encoding="utf-8"))
! 457 except Exception:
! 458 metadata = {}
459 entries.append(CacheEntry(key=child.name, root=self._root, metadata=metadata))
460 return entries
461
462 def _load_entry(self, key: str) -> Optional[CacheEntry]:Lines 464-473 464 if not entry.metadata_path.exists() or not entry.artifact_path.exists():
465 return None
466 try:
467 metadata = json.loads(entry.metadata_path.read_text(encoding="utf-8"))
! 468 except Exception:
! 469 metadata = {}
470 entry.metadata = metadata
471 return entry
472
473 def _touch(self, entry: CacheEntry) -> None:Lines 507-521 507 entry = self._fetch(cache_key)
508 if not entry:
509 return None
510 if verbose:
! 511 log.info(
512 "Simulation cache hit for workflow '%s'; using local results.", workflow_type
513 )
514
515 return entry
! 516 except Exception:
! 517 log.error("Failed to fetch cache results.")
518
519 def store_result(
520 self,
521 stub_data: WorkflowDataType,Lines 531-539 531 try:
532 simulation_obj = getattr(stub_data, "simulation", None)
533 simulation_hash = simulation_obj._hash_self() if simulation_obj is not None else None
534 if not simulation_hash:
! 535 return
536
537 version = _get_protocol_version()
538
539 cache_key = build_cache_key(Lines 557-566 557 task_id=task_id, # keeps a reverse link for legacy fetch_by_task
558 source_path=Path(path),
559 metadata=metadata,
560 )
! 561 except Exception:
! 562 log.error("Could not store cache entry.")
563
564
565 def _copy_and_hash(
566 source: Path, dest: Optional[Path], existing_hash: Optional[str] = NoneLines 612-620 612 return datetime.now(timezone.utc).isoformat()
613
614
615 def _timestamp_suffix() -> str:
! 616 return datetime.now(timezone.utc).strftime("%Y%m%d%H%M%S%f")
617
618
619 class _Hasher:
620 def __init__(self):Lines 628-636 628
629
630 def clear() -> None:
631 """Remove all cache entries."""
! 632 get_cache().clear()
633
634
635 def _canonicalize(value: Any) -> Any:
636 """Convert value into a JSON-serializable object for hashing/metadata."""Lines 640-658 640 str(k): _canonicalize(v)
641 for k, v in sorted(value.items(), key=lambda item: str(item[0]))
642 }
643 if isinstance(value, (list, tuple)):
! 644 return [_canonicalize(v) for v in value]
645 if isinstance(value, set):
! 646 return sorted(_canonicalize(v) for v in value)
647 if isinstance(value, Enum):
! 648 return value.value
649 if isinstance(value, Path):
! 650 return str(value)
651 if isinstance(value, datetime):
! 652 return value.isoformat()
653 if isinstance(value, bytes):
! 654 return value.decode("utf-8", errors="ignore")
655 return value
656
657
658 def build_cache_key( |
…local-cache-for-simulation-results-hashed-by-simulation-runtime-context # Conflicts: # tests/test_web/test_simulation_cache.py # tidy3d/web/api/autograd/autograd.py # tidy3d/web/api/container.py # tidy3d/web/api/webapi.py # tidy3d/web/cache.py
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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
Add opt-in local cache for simulation results hashed by simulation + runtime context
Background
Every repeated run of the same simulation re-uploads/downloads data. We want to cache the resulting .hdf5 locally (opt-in) and reuse it on identical runs. Cache key must go beyond _hash_self() because solver version, workflow path, environment, and runtime options also influence the artifact we return.
Greptile Overview
Updated On: 2025-10-07 11:35:12 UTC
Summary
This PR introduces a comprehensive opt-in local caching system for Tidy3D simulation results to eliminate redundant uploads and downloads when running identical simulations. The cache system stores simulation result HDF5 files locally using a composite cache key that combines simulation hash with runtime context (solver version, workflow type, environment variables) to ensure cache validity beyond simple simulation parameters.The implementation centers around a new
SimulationCacheclass intidy3d/web/cache.pythat provides thread-safe LRU caching with configurable size and entry limits. The cache integrates seamlessly into the existing web API workflow by interceptingrun()calls early to check for cached results and storing successful simulation outputs in theload()function. Cache entries are validated using SHA256 checksums and support atomic file operations to prevent corruption during concurrent access.Configuration is handled through a new
SimulationCacheSettingsclass inconfig.pywith sensible defaults (disabled by default, 10GB max size, 128 max entries,~/.tidy3d/cache/simulationsdirectory). The feature is exposed through an optionaluse_cacheparameter across all API entry points (run(),run_async(), autograd functions) allowing per-call override of global cache settings.The cache system handles both individual
Jobobjects and batch operations throughBatchobjects, with comprehensive error handling for edge cases like cache corruption, missing files, and network failures. The implementation follows enterprise software patterns with proper thread synchronization using a global singleton pattern and LRU eviction policies to manage cache size.Important Files Changed
Changed Files
Confidence score: 4/5
Sequence Diagram
sequenceDiagram participant User participant WebAPI as "web.run()" participant Cache as "SimulationCache" participant Job as "Job" participant Server as "Server" participant Stub as "Tidy3dStub" User->>WebAPI: "run(simulation, use_cache=True)" WebAPI->>Cache: "resolve_simulation_cache(use_cache)" Cache-->>WebAPI: "SimulationCache instance or None" alt Cache enabled WebAPI->>Cache: "try_fetch(simulation)" Cache->>Cache: "build_cache_key(simulation_hash, workflow_type, version)" Cache->>Cache: "_fetch(cache_key)" alt Cache hit Cache-->>WebAPI: "CacheEntry" WebAPI->>Cache: "materialize(path)" Cache-->>WebAPI: "Cached data path" WebAPI->>Stub: "postprocess(path)" Stub-->>WebAPI: "SimulationData" WebAPI-->>User: "SimulationData (from cache)" else Cache miss Cache-->>WebAPI: "None" WebAPI->>Job: "upload(simulation)" Job->>Server: "Upload simulation" Server-->>Job: "task_id" WebAPI->>Job: "start(task_id)" Job->>Server: "Start simulation" WebAPI->>Job: "monitor(task_id)" Job->>Server: "Poll status" Server-->>Job: "Status updates" WebAPI->>Job: "download(task_id, path)" Job->>Server: "Download results" Server-->>Job: "Simulation data file" WebAPI->>Stub: "postprocess(path)" Stub-->>WebAPI: "SimulationData" WebAPI->>Cache: "store_result(stub_data, task_id, path, workflow_type)" Cache->>Cache: "_store(cache_key, task_id, source_path, metadata)" WebAPI-->>User: "SimulationData" end else Cache disabled WebAPI->>Job: "upload(simulation)" Job->>Server: "Upload simulation" Server-->>Job: "task_id" WebAPI->>Job: "start(task_id)" Job->>Server: "Start simulation" WebAPI->>Job: "monitor(task_id)" Job->>Server: "Poll status" Server-->>Job: "Status updates" WebAPI->>Job: "download(task_id, path)" Job->>Server: "Download results" Server-->>Job: "Simulation data file" WebAPI->>Stub: "postprocess(path)" Stub-->>WebAPI: "SimulationData" WebAPI-->>User: "SimulationData" end