Simulation.run_time accepts RunTimeSpec

[tylerflex]

Took a first stab at this. It's a bit messy and there are some edge cases but seems to work. We should discuss before I do anything more to it.

Note: the evaluated run_time is now Simulation._run_time so there will be bugs unless we change this in eg. the backend. I think I got all the instances in the front end.

[tylerflex]

. Apart from that, and as part of testing, we should sprinkle this around in a bunch of notebooks. I feel like in some cases it will be quite long because of the default Q=100, but not sure.

I'm also curious how it will compare in some of the notebooks vs. the run_time we have hardcoded in there.

[tylerflex]

It also occurred to me that this will potentially conflict with #1473 so just a note to self that the adjoint run_time calculation will probably need to also change to _run_time or it will fail. (doesn't apply to 2.6*)

[tylerflex]

@momchil-flex @lucas-flexcompute I tested this on a few notebooks. Tried to pick a diverse range of situations. The results were kind of interesting.

notebook	run_time (set)	run_time (spec, evaluated)	Resonant?
BilayerSiNEdgeCoupler	3e-12	1.4e-10	no
CavityFOM	5.5e-12	5.0e-12	yes
DielectricMetasurfaceAbsorber	3e-10	6.7e-10	yes
ZeroCrossTalkTE	5e-12	1.4e-10	no

In general, it seems to over estimate the run time by about 1e2 for non-resonant devices.. but does pretty well for the resonant ones. Kind of as we expected given a default Q value of 100. Maybe this doesn't bode well for the usefulness of the run_time spec? at least as a default value for Simulation.run_time. We might need to encourage users to manually set the Q-factor, or even make it required in the RunTimeSpec().

EDIT: you can see my tests here: https://github.com/flexcompute/tidy3d-notebooks/compare/develop...tyler/run_time_spec

[lucas-flexcompute]

Or set the default Q to 1 if we want to provide a default at all.

[momchil-flex]

In general, it seems to over estimate the run time by about 1e2 for non-resonant devices.. but does pretty well for the resonant ones.

I guess it just so happens that the resonant ones have characteristic Qs of about 100?

Or set the default Q to 1 if we want to provide a default at all.

Yeah so that's what I kinda thought we would do at the start of this, i.e. have a good estimate for feed-forward, and then ask user to give us a Q estimate if it's not feed-forward. Or like use a default of like 3 (or a "safety factor" of 3x) to make sure we don't underestimate but also we don't way overestimate in the feedforward case.

We might need to encourage users to manually set the Q-factor, or even make it required in the RunTimeSpec().

Making it required would definitely be safer, just a little more annoying - but maybe still worth it?

[tylerflex]

Maybe a safe, default value of 1-10 would make sense. But I do worry about people getting confused if they have resonant devices.

Also a major advantage of this (in my mind) would be to make the Simulation.run_time optional, but perhaps we can't have that until we have some notion of auto-shutoff without a run_time maximum (or a very high value).

[tylerflex]

until then, this would basically just be a convenient way to estimate the run time, perhaps that's already useful enough, even if it doesn't replace the run time as a default.

[momchil-flex]

Also a major advantage of this (in my mind) would be to make the Simulation.run_time optional, but perhaps we can't have that until we have some notion of auto-shutoff without a run_time maximum (or a very high value).

I forget all our discussions about this, but I don't think we'd ever have something like this by default, because of the danger of it running way too long in some cases? So even if we implement something like this I would still think it would be a "use at your own risk" kinda thing.

[tylerflex]

Alright, so let's try to wrap this up for 2.7 or just close it. I think based on the discussions above, we've come to the following conclusions:

1. The optimal RunTimeSpec.q_factor seems to be pretty problem-dependent. A default value of 1-10 (maybe 3?) seems to cover most use cases, but for resonant devices, it will underestimate run_time quite a bit. Our options are: a. Make this a required parameter until we understand more? b. Set the default to ~3 and hope that through our use of this in the docs, users realize they should set it higher for resonant devices? c. Set the default to 100 and just deal with overestimating run_time as a safer option.

2. The Simulation.run_time default can only be set as a RunTimeSpec if we choose a default for that (1b or 1c) so our options are a. keep this required. b. set it to the default RunTimeSpec.

Maybe my preference is to keep both q_factor and run_time required (1a, 2a) until we test this more. And then we can just roll it out with some usage scattered throughout the notebooks (eg. in the ones I tested with earlier)

Thoughts?

[momchil-flex]

So to clarify what you are proposing is keep run_time required, and keep q_factor required in RunTimeSpec? I think I'm fine with this.

[tylerflex]

So to clarify what you are proposing is keep run_time required, and keep q_factor required in RunTimeSpec? I think I'm fine with this.

Yea basically the most conservative option, because why not.. if people like using this feature (and I think it's actually more convenient than setting run_time in a lot of cases) then we can enhance it later or make it more streamlined to use

[momchil-flex]

Yeah, let's do that.

[tylerflex]

k, got this ready to go. Not sure if there's an associated backend commit?