Add adaptive scientific notation for frequency range

[rahul-flex]

As discussed with @tomflexcompute we want to include as many significant digits as possible, provided the two numbers (frequency range: fmin_med and fmax_med) remain identical up to that point.
Once a differing digit is encountered, the output rounds off subsequent digits.
Also, the function returns the min and max values with same exponent.

Below are few test examples:
inputs are fmin_med and fmax_med

`Input: 1234567.56, 1234577.34
Output: 1.23457e6, 1.23458e6

Input: 1234567.56, 211234577.34
Output: 1e6, 211e6

Input: 1234567, 1934577
Output: 1e6, 2e6

Input: 1234567, 1284577
Output: 1.2e6, 1.3e6

Input: 123, 128
Output: 1.2e2, 1.3e2

Input: 0.00123, 0.00128
Output: 1.2e-3, 1.3e-3

Input: 0, 10
Output: 0e+00, 1e+01

Input: 123456789012345, 123456789012346
Output: 1.23456789012345e14, 1.23456789012346e14`

[tomflexcompute]

@rahul-flex actually I think we should at least display a certain number of digitals like 5 for example. Since in most materials, the f_min and f_max differ in the first significant digital already. Could you test it on the test model and see how the output warning looks like?

[rahul-flex]

@tomflexcompute The warning message for the test setup you provided looks like this:

18:15:58 EST WARNING: The medium associated with structures[0] has a frequency  
range: (1e12, 250e12) (Hz) that does not fully cover the           
frequencies contained in monitors[0]. This can cause inaccuracies  
in the recorded results. 

[tomflexcompute]

Yeah I think we do need more significant digitals otherwise the numbers are too crude.

Fyi this pr is to address #1086

[momchil-flex]

I guess this shouldn't be a Simulation level method. Maybe Tidy3dBaseModel? Let's also change the variable names to not specifically refer to frequency, more like min_val, max_val? And then regarding @tomflexcompute's point, add an extra argument like min_digits=4? Looking at the code it seems that this number would refer not to the total number of significant digits, but to the digits after the decimal, e.g. 1.1234e12 to 250.1234e12 right? I think that's fine.

[yaugenst-flex]

agree with Tidy3dBaseModel (here). also decorate this with a staticmethod and change the below calls to self._scientific_notation instead of cls._scientific_notation, otherwise this will error when called on a class instance.

[yaugenst-flex]

Thanks @rahul-flex this looks pretty good overall. Don't forget to add a unit test and a changelog entry though, this is a nicely testable function.

[yaugenst-flex]

agree with Tidy3dBaseModel (here). also decorate this with a staticmethod and change the below calls to self._scientific_notation instead of cls._scientific_notation, otherwise this will error when called on a class instance.

[yaugenst-flex]

Edge case: this will cause an infinite loop if normalized_fmin == normalized_fmax. Not very likely to happen, but should be handled

[notion-workspace]

🧑‍🔬 Scientific notation for frequency in plots

[rahul-flex]

Thanks, @momchil-flex, @tomflexcompute, and @yaugenst-flex for the review comments. I have updated the function and added test cases.
@momchil-flex Yes, the min_digits refers to the digits after the decimal in scientific notation.

[tomflexcompute]

Thanks @rahul-flex I think it should work very well.

[yaugenst-flex]

Thanks @rahul-flex almost good to go :)

[yaugenst-flex]

Thanks @rahul-flex the test is great, only request I have is could you turn this into a parameterized test instead? That makes them run as separate tests so it will be easier to debug in case something fails. Also note that you generally don't need to add assertion messages in pytest because they are a bit redundant as you will get a summary of the failed tests and their parameters.

Here is how to convert to a parameterized test:

@pytest.mark.parametrize(
    "min_val, max_val, min_digits, expected",
    [
        (1234567, 1234577, 4, ("1.23457e6", "1.23458e6")),
        (1234567, 1234577, 6, ("1.234567e6", "1.234577e6")),
        (1.23e-3, 1.28e-3, 4, ("1.2300e-3", "1.2800e-3")),
        (123456789012345, 123456789012346, 4, ("1.23456789012345e14", "1.23456789012346e14")),
        (123656789012345, 123756789012346, 4, ("1.2366e14", "1.2376e14")),
        (123, 123, 4, ("1.2300e2", "1.2300e2")),
    ],
    ids=[
        "default_min_digits",
        "increased_min_digits",
        "small_numbers",
        "large_numbers_precise",
        "large_numbers_rounded",
        "identical_numbers"
    ]
)
def test_scientific_notation(min_val, max_val, min_digits, expected):
    """Test the _scientific_notation method with various inputs."""
    result = Tidy3dBaseModel._scientific_notation(min_val, max_val, min_digits=min_digits)
    assert result == expected

[yaugenst-flex]

Simulation is a subclass of Tidy3dBaseModel, so just call self._scientific_notation() here

[yaugenst-flex]

Nice, looks great @rahul-flex!

[momchil-flex]

Looks good after small changelog comment, and eventually squashing all commits.

[momchil-flex]

This changelog item should be in Unreleased not in 2.8.0rc1. Also generally we tend to put new things at the bottom of the list (Added/Fixed/Changed), not at the top.