Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Adding num_proc arg to adjoint run_local, and properly avoiding validators #1686

Merged
merged 1 commit into from May 13, 2024
Merged

Adding num_proc arg to adjoint run_local, and properly avoiding validators #1686

merged 1 commit into from May 13, 2024

Conversation

momchil-flex
Copy link
Collaborator

No description provided.

tylerflex
tylerflex requested changes May 7, 2024
View reviewed changes
Copy link
Collaborator

@tylerflex tylerflex left a comment

Choose a reason for hiding this comment

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

Couple minor things but overall looks good. Think it requires a changelog fixed item?

tests/test_plugins/test_adjoint.py Show resolved Hide resolved
tidy3d/plugins/adjoint/web.py Outdated Show resolved Hide resolved
tylerflex
tylerflex approved these changes May 8, 2024
View reviewed changes
Copy link
Collaborator

@tylerflex tylerflex left a comment

Choose a reason for hiding this comment

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

looks good, will leave it up to you regarding changelog. maybe since the skipped validators were introduced in 2.7 rc1 (I think?) we can just leave it out as it will be fixed in 2.7

@tylerflex
Copy link
Collaborator

tylerflex commented May 8, 2024 •
edited

@prashkh It seems (based on the tests) that on python3.9, all the results are None. Would you mind looking into this? Im not really sure how to proceed. Perhaps there is some error that is getting caught by the try/except and setting the results to None? or potentially the mode solvers are never getting run?

@tylerflex
Copy link
Collaborator

it's possible when I tried to recover your changes, I missed some detail?

@prashkh
Copy link

prashkh commented May 8, 2024 •
edited

@tylerflex This PR is for the test_adjoint right? or are you talking about run_batch addition for the web mode solver?

@tylerflex
Copy link
Collaborator

Yea sorry, wrong PR.. supposed to be commenting on this one #1685 :D

@tylerflex tylerflex added rc2 2nd pre-release 2.7 will go into version 2.7.* labels May 13, 2024
@tylerflex tylerflex merged commit 6316bad into pre/2.7 May 13, 2024
16 checks passed
@tylerflex tylerflex deleted the momchil/adjoint_run_local branch May 13, 2024 14:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tylerflex tylerflex tylerflex approved these changes

Assignees
No one assigned
Labels
2.7 will go into version 2.7.* rc2 2nd pre-release
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@momchil-flex @tylerflex @prashkh