Skip to content

maint[tests]: reduce runtime of most expensive tests #2403

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

Draft
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

yaugenst-flex
Copy link
Collaborator

Shaves a good amount of seconds off from the slowest running tests. Introduces a fixture that allows disabling most of the td.Simulation validation. Some of these are still slower than they should be probably but it's a start.

Before:

75.52s call     tests/test_plugins/test_design.py::test_sweep[sweep_method0]
40.89s call     tests/test_components/test_autograd.py::test_autograd_async[<ALL>-diff]
34.13s call     tests/test_components/test_autograd.py::test_autograd_async[<ALL>-field_vol]
33.85s call     tests/test_components/test_simulation.py::test_num_lumped_elements
31.11s call     tests/test_components/test_autograd.py::test_autograd_async[<ALL>-mode]
24.55s call     tests/test_components/test_autograd.py::test_autograd_async[<ALL>-field_point]
22.17s call     tests/test_plugins/test_adjoint.py::test_adjoint_pipeline[True]
20.26s call     tests/test_components/test_scene.py::test_num_mediums
19.62s call     tests/test_components/test_IO.py::test_validation_speed
19.13s call     tests/test_components/test_autograd.py::test_autograd_async_server[<ALL>-diff]

After:

42.02s call     tests/test_components/test_simulation.py::test_num_lumped_elements
24.76s call     tests/test_plugins/test_adjoint.py::TestJaxComplexPolySlab::test_vertices_grads[-0.02-10-3-base_vertices1]
24.61s call     tests/test_components/test_autograd.py::test_autograd_async_server[<ALL>-diff]
23.64s call     tests/test_plugins/test_adjoint.py::test_adjoint_pipeline[True]
23.14s call     tests/test_components/test_autograd.py::test_autograd_async_some_zero_grad[<ALL>-field_vol]
22.75s call     tests/test_components/test_autograd.py::test_autograd_async[<ALL>-diff]
22.29s call     tests/test_components/test_eme.py::test_eme_sim_data
21.59s call     tests/test_components/test_autograd.py::test_autograd_async_some_zero_grad[<ALL>-mode]
21.50s call     tests/test_components/test_scene.py::test_num_mediums
21.35s call     tests/test_components/test_autograd.py::test_autograd_async_server[<ALL>-field_vol]

Note: Not sure where the 42s spent on test_num_lumped_elements comes from, might have gotten unlucky on an efficiency core.

@yaugenst-flex yaugenst-flex self-assigned this Apr 23, 2025
@yaugenst-flex yaugenst-flex marked this pull request as ready for review April 23, 2025 12:06
@momchil-flex
Copy link
Collaborator

Interesting.

  • Why does validation take so long in the test where it does?
  • I thought that there was no good "construct" method that works right off the bat? Like I thought that it doesn't initialize embedded models, you get something like e.g. Simulation looks like the proper object but an embedded e.g. Monitor would look like a dict rather than a Monitor.

@yaugenst-flex yaugenst-flex marked this pull request as draft April 23, 2025 12:36
@yaugenst-flex
Copy link
Collaborator Author

  • Why does validation take so long in the test where it does?

The test_design.py::test_sweep was just assembling ~700 simulations, and validating those amounted to ~20% of the total time. I also reduced the size of the sweep, that was probably a bigger speedup.

  • I thought that there was no good "construct" method that works right off the bat? Like I thought that it doesn't initialize embedded models, you get something like e.g. Simulation looks like the proper object but an embedded e.g. Monitor would look like a dict rather than a Monitor.

At least in the tests we are generally passing already-validated submodels, and in that case construct() works fine. It's just a problem if you're passing dicts around I think.

Anyways, this PR is not done I only undrafted it to check out the CI. I'll keep this one open for a while and maybe we can speed up the tests a bit more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants