flexcompute / tidy3d

Fast electromagnetic solver (FDTD) at scale.
https://docs.flexcompute.com/projects/tidy3d/en/latest/
GNU Lesser General Public License v2.1
175 stars 40 forks source link

Remove `assert`s from non-test code #1833

Closed yaugenst-flex closed 1 month ago

yaugenst-flex commented 1 month ago

We currently have a bunch of assert statements scattered throughout the core code. Would be good to get rid of these in all production code since this can change the behavior of tidy3d when run with python -O. Even if we never use that flag, it also affects downstream packages that might want to use it.

Once done, we probably want to enable S101, or even the rest of the flake8-bandit ruleset.

Here is a quick grep of where we are currently using assert:

components/grid/grid.py
520:            assert pt_min <= pt_max, "min point was greater than max point"

components/simulation.py
1261:            assert get_bounds(geom, axis)[0] == get_bounds(geom, axis)[1]

components/geometry/base.py
682:        assert len(xyz_filtered) == 1, "exactly one kwarg in [x,y,z] must be specified."
3219:            assert len(grad_vjp_values) == 1, "Got multiple gradients for single geometry field."

components/geometry/polyslab.py
1376:        assert derivative_info.paths == [
1400:        assert edge_centers_xyz.shape == (num_vertices, 3), "something bad happened"

web/api/material_fitter.py
76:        assert fitter
77:        assert options

web/api/autograd/autograd.py
658:        assert len(frequencies) == 1, "Multiple adjoint freqs found"

plugins/smatrix/ports/rectangular_lumped.py
197:        assert orth_index > 0
198:        assert inject_center < h_coords_along_injection[orth_index]
199:        assert h_coords_along_injection[orth_index - 1] < inject_center

plugins/smatrix/component_modelers/base.py
248:        assert a_matrix.dims == b_matrix.dims

plugins/microwave/custom_path_integrals.py
127:            assert isinstance(em_field, ModeSolverData)

plugins/microwave/path_integrals.py
126:            assert isinstance(scalar_field, ScalarModeFieldDataArray)

plugins/dispersion/fit.py
170:        assert len(coeffs) % 4 == 0, "len(coeffs) must be multiple of 4."
695:        assert len(data.shape) == 2, "data must contain [wavelength, ndata, kdata] in columns"
696:        assert data.shape[-1] in (2, 3), "data must have either 2 or 3 rows (if k data)"
momchil-flex commented 1 month ago

Definitely, these must be leftover from a long time ago or have snuck in, because in principle we flag those in PR reviews. But yeah actually catching that automatically also sounds good.