FrequencySimulationResult, FrequencySimulation, and basis order

[arturgower]

This is just to debate out current types.

Am not sure how useful is the type FrequencySimulation now. Essentially it is not just an added step were the user always has to call:

sim = FrequencySimulation(particles, source)
result = run(sim, x, ω)

Why not just have:

result = run(particles, source, x, ω)

how is FrequencySimulation useful?

Also for debate, I think FrequencySimulationResult should have a field basis_order::Vector{Int}. I think by having the field basis_order it makes it easier for new users to discover this option, rather than just being keyword of run. Second, even if the used doesn't specify basis_order it can still be very important to know and keep a record of what the basis order was. On the other hand, if FrequencySimulationResult didn't have the field basis_order that would allow us more freedom to, for example, have a different basis order for each particle, depending on it's radius....

[jondea]

The idea with the FrequencySimulation struct was so that you could build it up in several steps. But I agree with you, in practice it is not natural to do this, especially with the removal of the background medium. I'm in favour of removing it, although I will say that I think the run function would need a new name, perhaps runsimulation? Or even something implying you are generating results or perhaps response?

I think it would be very strange for the FrequencySimulationResult to have the basis_orders, but not say, the particles or the source. Especially if the basis order was a vector, how would you know which basis order corresponded to each particle? The good thing about FrequencySimulationResult is that it is very simple, but contains all the necessary information to plot in space and frequency or convert to a TimeSimulationResult, but nothing extra. For discoverability perhaps we could add some output which says what the basis order is when we run the simulation (maybe on by default but can be turned off with a flag or an output level?). I also agree that we may want different orders for different particles, or even xs and ωs. We may also want an automatic and adaptive way to choose the order.

[arturgower]

Agreed we leave FrequencySimulationResult alone. For now I've just made

result = run(particles, source, x, ω)

as a call to

sim = FrequencySimulation(particles, source)
result = run(sim, x, ω)

Most packages have an elaborate type that carries all sorts of information about the simulation, i.e. things like basis_order. It avoids having to use lots of optional calls to run, and can help teach the user about different options of say run. How about we build FrequencySimulation by calling FrequencySimulation(particles, source,x,ω) that way FrequencySimulation can have carry all options, even those that depend on known ω, such as basis_order? Just an idea, happy to discuss alternatives.

I don't think the name run should change, as it still does the same thing. Also, any run can be thought of as running some simulation.

[arturgower]

And yes, for discoverability, we should have run, or FrequencySimulation output the basis order and other needed info. I also think that an adaptive basis order for every particle is defo the way to go. The lack of this feature often causes me pain.

For example, if you try to have a massive and tiny particle together, if the basis order is too low we lose a lot of accuracy, if the basis order is too high, we actually get numerical instability because of the tiny particle. And adaptive is the only real way to get the right basis order for each particle.

[jondea]

I think this all makes sense :smile:

One note is that we just need to make sure we don't accidentally overwrite base methods of run.