Simulation.run_time accepts RunTimeSpec #1495

tylerflex · 2024-02-23T16:35:24Z

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.

lucas-flexcompute · 2024-02-23T18:25:35Z

tidy3d/components/simulation.py

+    @cached_property
+    def n_max(self) -> float:
+        """Maximum refractive index in the ``Simulation``."""
+        eps_max = max(abs(struct.medium.eps_model(self.freq_max)) for struct in self.structures)


Is there any reason to use the maximum frequency instead of the central one? It is possible that the maximum index is found at lower frequencies, no?
Also, if you are looking for max(n), that should be max(Re{sqrt(ε)}), not Re{sqrt(argmax(|ε|))}.

Hm, so this was basically refactored out of our previous wvl_min_material, which grabbed the minimum wavelength in the material. I guess I should probably do something like take the central frequency or average over frequencies.

Also, if you are looking for max(n), that should be max(Re{sqrt(ε)}), not Re{sqrt(argmax(|ε|))}.

I wonder also if this means our previous function was wrong.

Is this function used anywhere? In the mesher we just use eps_complex_to_nk, which is probably best so you don't have to think about it.

tidy3d/tidy3d/components/grid/mesher.py

Line 437 in 89d9d43

n, k = structure.medium.eps_complex_to_nk(

Note that if you want to average over freqs we may be running in the issue that not all eps_model-s may work for a list of freqs. #1290

Personally I feel like using the central frequency may be fine. This is all going to be approximate in various ways anyway.

ok, let's use central frequency and I'll just use eps_complex_to_nk(eps_model(source_time.freq0))

I made some changes here: maybe double check them to see if they make sense (they affect wvl_mat_min too. https://github.com/flexcompute/tidy3d/pull/1495/files

momchil-flex

Thanks! Some small comments. 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.

momchil-flex · 2024-02-23T19:07:43Z

tidy3d/components/source.py

+    def end_time(self) -> float | None:
+        """Time after which the source is effectively turned off / close to zero amplitude."""
+        if not self.remove_dc_component:
+            return None


I see your thinking here in that the DC signal will remain, but I think the same end_time could apply regardless. For example, frequency domain fields should still be correct after that end time, except at very low frequencies.

yea true. maybe I'm confused as to why the user would want to not remove DC component? is it more of an internal thing?

It's in cases where you do want to simulate low frequencies, or rather a broadband pulse that also includes low frequencies. I think mostly relevant for RF. But yeah even if you turn it off you don't want to run indefinitely (the DC component won't decay anyway...) you just might need to do some extra normalization for the low-frequency components.

momchil-flex · 2024-02-23T19:07:50Z

tidy3d/components/simulation.py

-    run_time: pydantic.PositiveFloat = pydantic.Field(
-        ...,
+    run_time: pydantic.PositiveFloat | RunTimeSpec = pydantic.Field(
+        RunTimeSpec(),


I'm a little nervous setting this as default - I'd rather keep it a required field and we can just add it to various examples?

I'm fine with that

eventually it would be nice to be able to just not specify anything for run_time but I think for the testing phase we can make it required and see how it goes.

Yeah cause we do charge a minimum of 10% of estimated cost, so if the run time is often way overestimated, it may not be good for users.

momchil-flex · 2024-02-23T19:08:09Z

tidy3d/components/simulation.py

                    )

    """ Accounting """

+    @cached_property
+    def _run_time(self) -> float:


Need to check if run_time is used on the backend and replace.

yea, should I give that a shot or do you want to?

I actually didn't see anything in tidy3d-core (at least on the python side) that uses .run_time explicitly. (or that seemed like it should use _run_time. So maybe it's best you take a look

I found one place

momchil-flex · 2024-02-23T19:13:46Z

tidy3d/components/run_time_spec.py

+    )
+
+    quality_factor: pd.PositiveFloat = pd.Field(
+        100,


@lucas-flexcompute does this value mean that for a pretty feed-forward simulation, say a directional coupler, the run time will be overestimated by about a factor of 100?

tylerflex · 2024-02-26T15:03:57Z

. 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 · 2024-02-26T15:16:41Z

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 · 2024-02-27T13:59:44Z

@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:
flexcompute/tidy3d-notebooks@develop...tyler/run_time_spec

lucas-flexcompute · 2024-02-27T14:32:13Z

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

momchil-flex · 2024-02-27T22:01:54Z

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 · 2024-02-27T22:09:42Z

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 · 2024-02-27T22:10:35Z

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 · 2024-02-27T22:20:53Z

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 · 2024-04-02T01:40:37Z

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:

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.
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 · 2024-05-01T00:27:22Z

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 · 2024-05-01T00:36:21Z

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 · 2024-05-01T16:36:31Z

Yeah, let's do that.

tylerflex · 2024-05-01T16:58:08Z

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

…ally based on simulation properties.

tylerflex added the 2.7 will go into version 2.7.* label Feb 23, 2024

tylerflex self-assigned this Feb 23, 2024

tylerflex linked an issue Feb 23, 2024 that may be closed by this pull request

Convenience for setting simulation run time #1267

Closed

tylerflex marked this pull request as draft February 23, 2024 16:36

tylerflex force-pushed the tyler/run_time_spec branch 2 times, most recently from 7867ca7 to 6302962 Compare February 23, 2024 16:45

tylerflex requested review from momchil-flex and lucas-flexcompute February 23, 2024 16:54

lucas-flexcompute reviewed Feb 23, 2024

View reviewed changes

tylerflex force-pushed the tyler/run_time_spec branch from 6302962 to 7c42cce Compare February 23, 2024 18:42

momchil-flex reviewed Feb 23, 2024

View reviewed changes

momchil-flex removed the 2.7 will go into version 2.7.* label Mar 29, 2024

tylerflex force-pushed the tyler/run_time_spec branch from 3b76859 to b8b3803 Compare April 2, 2024 01:56

tylerflex marked this pull request as ready for review April 2, 2024 01:56

tylerflex added the 2.7 will go into version 2.7.* label Apr 10, 2024

tylerflex force-pushed the tyler/run_time_spec branch 3 times, most recently from f49906d to 492a10a Compare May 1, 2024 16:57

tylerflex added the rc2 2nd pre-release label May 1, 2024

tylerflex force-pushed the tyler/run_time_spec branch from 492a10a to b33c233 Compare May 1, 2024 20:45

Simulation.run_time accepts RunTimeSpec to set run time automatic…

45c98f6

…ally based on simulation properties.

tylerflex force-pushed the tyler/run_time_spec branch from b33c233 to 45c98f6 Compare May 2, 2024 20:33

tylerflex merged commit 05cd0e4 into pre/2.7 May 2, 2024
16 checks passed

tylerflex deleted the tyler/run_time_spec branch May 2, 2024 21:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Simulation.run_time accepts RunTimeSpec #1495

Simulation.run_time accepts RunTimeSpec #1495

tylerflex commented Feb 23, 2024 •

edited

Loading

lucas-flexcompute Feb 23, 2024

tylerflex Feb 23, 2024

momchil-flex Feb 23, 2024 •

edited

Loading

tylerflex Feb 23, 2024

tylerflex Feb 23, 2024

momchil-flex left a comment

momchil-flex Feb 23, 2024

tylerflex Feb 26, 2024

momchil-flex Feb 26, 2024

momchil-flex Feb 23, 2024

tylerflex Feb 26, 2024

tylerflex Feb 26, 2024

momchil-flex Feb 26, 2024

momchil-flex Feb 23, 2024

tylerflex Feb 26, 2024

tylerflex Feb 26, 2024

momchil-flex Feb 26, 2024

momchil-flex Feb 23, 2024

tylerflex commented Feb 26, 2024

tylerflex commented Feb 26, 2024

tylerflex commented Feb 27, 2024 •

edited

Loading

lucas-flexcompute commented Feb 27, 2024

momchil-flex commented Feb 27, 2024

tylerflex commented Feb 27, 2024

tylerflex commented Feb 27, 2024

momchil-flex commented Feb 27, 2024

tylerflex commented Apr 2, 2024 •

edited

Loading

momchil-flex commented May 1, 2024

tylerflex commented May 1, 2024

momchil-flex commented May 1, 2024

tylerflex commented May 1, 2024

Simulation.run_time accepts RunTimeSpec #1495

Simulation.run_time accepts RunTimeSpec #1495

Conversation

tylerflex commented Feb 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

momchil-flex Feb 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

momchil-flex left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tylerflex commented Feb 26, 2024

tylerflex commented Feb 26, 2024

tylerflex commented Feb 27, 2024 • edited Loading

lucas-flexcompute commented Feb 27, 2024

momchil-flex commented Feb 27, 2024

tylerflex commented Feb 27, 2024

tylerflex commented Feb 27, 2024

momchil-flex commented Feb 27, 2024

tylerflex commented Apr 2, 2024 • edited Loading

momchil-flex commented May 1, 2024

tylerflex commented May 1, 2024

momchil-flex commented May 1, 2024

tylerflex commented May 1, 2024

tylerflex commented Feb 23, 2024 •

edited

Loading

momchil-flex Feb 23, 2024 •

edited

Loading

tylerflex commented Feb 27, 2024 •

edited

Loading

tylerflex commented Apr 2, 2024 •

edited

Loading