-
Notifications
You must be signed in to change notification settings - Fork 11
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
FrequencySimulationResult, FrequencySimulation, and basis order #29
Comments
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 I think it would be very strange for the |
Agreed we leave 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 I don't think the name |
And yes, for discoverability, we should have 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. |
I think this all makes sense 😄 One note is that we just need to make sure we don't accidentally overwrite base methods of |
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:Why not just have:
how is
FrequencySimulation
useful?Also for debate, I think
FrequencySimulationResult
should have a fieldbasis_order::Vector{Int}
. I think by having the fieldbasis_order
it makes it easier for new users to discover this option, rather than just being keyword ofrun
. Second, even if the used doesn't specifybasis_order
it can still be very important to know and keep a record of what the basis order was. On the other hand, ifFrequencySimulationResult
didn't have the fieldbasis_order
that would allow us more freedom to, for example, have a different basis order for each particle, depending on it's radius....The text was updated successfully, but these errors were encountered: