-
Notifications
You must be signed in to change notification settings - Fork 66
FXC-4439: removing devsim mentions #3058
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR removes all references to the DEVSIM dependency from the codebase and cleans up an unused parameter from the container monitoring functionality.
- Removes the
devsimpackage dependency frompyproject.toml - Updates outdated docstrings to reflect the current HeatCharge solver implementation
- Removes unused
postprocess_worker_groupparameter from theBatch.monitor()method
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
tidy3d/web/api/container.py |
Removes unused postprocess_worker_group parameter from the monitor method signature |
tidy3d/components/tcad/types.py |
Updates module docstring from "DEVSIM case" to "HeatCharge solver associated types" |
tidy3d/components/tcad/doping.py |
Updates module docstring from "DEVSIM case" to "Semiconductor doping definitions" |
pyproject.toml |
Removes devsim from optional dependencies and from dev, docs, and heatcharge extras |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
4 files reviewed, 1 comment
| vtk = ["vtk"] | ||
| ruff = ["ruff"] | ||
| heatcharge = ["trimesh", "vtk", "devsim"] | ||
| heatcharge = ["trimesh", "vtk"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Check that a changelog entry was added for this change - removing devsim from the heatcharge extras is user-facing
Context Used: Rule from dashboard - Require a changelog entry for any PR that is not purely an internal refactor. (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: pyproject.toml
Line: 224:224
Comment:
**style:** Check that a changelog entry was added for this change - removing `devsim` from the `heatcharge` extras is user-facing
**Context Used:** Rule from `dashboard` - Require a changelog entry for any PR that is not purely an internal refactor. ([source](https://app.greptile.com/review/custom-context?memory=b72c7231-b258-4d03-aed2-51a50a9fe757))
How can I resolve this? If you propose a fix, please make it concise.
Diff CoverageDiff: origin/develop...HEAD, staged and unstaged changesNo lines with coverage information in this diff. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
crazy how big that dependency was then
| download_on_success: bool = False, | ||
| path_dir: PathLike = DEFAULT_DATA_DIR, | ||
| replace_existing: bool = False, | ||
| postprocess_worker_group: Optional[str] = None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@momchil-flex used claude to do a historical search of related changes and it said this was what was leftover from those postprocess changes only
Also piggy-backing removal of one unused argument in the containers monitoring (left over from previous modeler code). @daquintero actrually could you take a look at that code and see if more stuff can be removed now that the postprocess function is integrated with the server call?
Greptile Overview
Greptile Summary
This PR removes all references to the DEVSIM library from the codebase. The changes include removing the devsim dependency from
pyproject.toml, updating module docstrings in TCAD components to use more descriptive language, and cleaning up an unusedpostprocess_worker_groupparameter from theBatch.monitor()method.Key changes:
devsimfrom optional dependencies and all extras groups (dev,docs,heatcharge)tidy3d/components/tcad/doping.pyandtypes.pyto be more domain-specificpostprocess_worker_groupparameter fromBatch.monitor()method incontainer.pypoetry.lockfile to reflect dependency changesAll changes are safe and straightforward cleanup operations.
Confidence Score: 5/5
Important Files Changed
File Analysis
postprocess_worker_groupparameter frommonitor()method - safe cleanupSequence Diagram
sequenceDiagram participant Dev as Developer participant Pyproject as pyproject.toml participant Container as web/api/container.py participant TCAD as tcad/*.py Dev->>Pyproject: Remove devsim dependency Note over Pyproject: Removed from dependencies,<br/>dev extras, docs extras,<br/>and heatcharge extras Dev->>Container: Remove unused parameter Note over Container: Removed postprocess_worker_group<br/>from monitor() signature Dev->>TCAD: Update docstrings Note over TCAD: Changed "DEVSIM case" to<br/>domain-specific descriptions