broadband source feature

[dbochkov-flexcompute]

frontend part of broadband sources

[momchil-flex]

This looks good overall, but I'd like for @tylerflex to have a look if it makes sense too.

[momchil-flex]

Let's just move this to the backend; put it in source.py as a standalone function for now, after this is merged I will do the change of implementing BackendSource types and will include it there.

[tylerflex]

Should this rather be pydantic.PositiveInt? What happens if the num_freqs = 0?

[tylerflex]

I think we could make this more readable by splitting it over a few lines and assigning some intermediate variables, for example:

f_add = 0.5 * (fmin + fmax)
f_diff = 0.5 * (fmax - fmin)
f_range = (2 * np.flip(np.arange(self.num_freqs)) + 1) / self.num_freqs
return f_add + f_diff * np.cos(0.5 * np.pi * f_range)

or something like this (double check..)

[tylerflex]

can we add the keyword argument explicitly? Ie

fmin, fmax = self.source_time.frequency_range(num_fwidth=4)

I had to look up what frequency_range accepted because I forgot, doing it this way will make it a bit more readable. (note that 4 is the default value, so alternatively we could just remove the argument here)

[tylerflex]

Looks good overall. Please add some front end tests as well.
In tests/test_components/test_source.py, we should add a test where each of the broadband sources are initialized with some num_freqs > 1 and then call these new methods map_frequencies and frequency_grid and make sure they work as expected. If you can think of other things to test (not involving any backend), feel free to add them as well. Thanks!

[tylerflex]

Thanks for adding the test and refactoring that method, looks great!

[momchil-flex]

I think we should take some care to prevent a naive user from specifying way too high num_freqs, which could make it slow to compute all modes. Some suggestions:

• Add an explanation to the description of the num_freqs field that Chebyshev decomposition is used which can approximate weakly-varying functions very well with a small number of points.
• Limit num_freqs to e.g. 100.
• Add a warning if the user sets e.g. num_freqs > 20, saying that num_freqs > 20 is not needed unless very sharp dependence of the mode profiles on frequency is expected.

What do you think?

[dbochkov-flexcompute]

I think we should take some care to prevent a naive user from specifying way too high num_freqs, which could make it slow to compute all modes. Some suggestions:

• Add an explanation to the description of the num_freqs field that Chebyshev decomposition is used which can approximate weakly-varying functions very well with a small number of points.
• Limit num_freqs to e.g. 100.
• Add a warning if the user sets e.g. num_freqs > 20, saying that num_freqs > 20 is not needed unless very sharp dependence of the mode profiles on frequency is expected.

What do you think?

I think that's a great suggestion, will add all of that

[momchil-flex]

Great, for point 2 you can directly add 'le' to the field initialization https://pydantic-docs.helpmanual.io/usage/schema/#field-customization

For 3 you'd need a small custom validator, have a look at other components if you haven't seen those in action yet.

[momchil-flex]

Sorry, very nitpicky, but the style we follow in python is snake_case for functions and CamelCase for class names only, so this would be test_broadband_source

[dbochkov-flexcompute]

sounds good, I guess I was just following test_FieldSource() and test_UniformCurrentSource()

[momchil-flex]

Also nitpicky, but I suggest slightly different wording, something like "A large number ({val}) of frequency points is used in a broadband source. This can slow down simulation time and is only needed if the mode fields are expected to have a very sharp frequency dependence. 'num_freqs' < 20 is sufficient in most cases."