Issue a warning when accessing unavailable group index data (#1169)

[lucas-flexcompute]

No description provided.

[tylerflex]

I think maybe tom should take a look too to make sure the warning message is sufficient. One thing I can imagine users will get caught up on is that they can't set

mode_spec.group_index_step = True

directly, and instead I might suggest something like

The group index was not computed. To calculate group index, pass `group_index_step = True` in the `ModeSpec`.

Also, does this require any backend changes? because now we'd need to load the n_group_raw into the ModeSolverDataset and ModeData?

[lucas-flexcompute]

Yes, this will require both schema a backend changes I believe. I'm just waiting to see if this is the route we prefer, instead of changing the defaults depending on the class as originally proposed

[tomflexcompute]

Thanks @lucas-flexcompute . I agree with Tyler's suggestion that a more descriptive message would be better especially for less experienced users. I personally wouldn't mind having group_index_step = False as default since the warning should help avoid confusion.

[momchil-flex]

I like this solution too. In fact I woke up at 3 or 4 am thinking that that's how it should be handled. :D

(By now I seem to have a constant background process running, thinking about Tidy3D...)

[tomflexcompute]

I like this solution too. In fact I woke up at 3 or 4 am thinking that that's how it should be handled. :D

(By now I seem to have a constant background process running, thinking about Tidy3D...)

I often think about how to answer customers' tricky questions at night in the background 🤯 , which is really bad for sleep quality. Hope your sleep quality is unaffected by the background process...

[lucas-flexcompute]

I like this solution too. In fact I woke up at 3 or 4 am thinking that that's how it should be handled. :D

(By now I seem to have a constant background process running, thinking about Tidy3D...)

Time for a short vacation, maybe? :P

[momchil-flex]

Should we just put this in 2.4.2 and not 2.5? Seems ready to me.

[tylerflex]

@momchil-flex doesn't the backend have to construct n_group when creating the ModeData and ModeSolverData? I would assume there needs to be some backend changes because now this field is called n_group_raw? it might be safer to wait for 2.5 but if it's urgent we can do it for 2.4.2, just want to check.

[momchil-flex]

It does but I'm building a separate solver for 2.4.2 anyway.

[tylerflex]

ok, any possible unintended consequences though? For example a monitor_data.hdf5 from 2.4.1 won't be able to be loaded in 2.4.2? my vote is probably to wait for 2.5 unless you're pretty confident it will be fine.

[momchil-flex]

Good point. Actually ideally we don't want for it to not be possible to load a 2.4 sim data in 2.5. Even this is kinda bad since someone upgrading won't be able to revisit their recent simulations. We're making a backwards-incompatible change to the SimulationData schema, effectively. Which is making me completely rethink this...

[momchil-flex]

I can't think of a way to warn without changing the SimulationData schema and I feel like I'd rather not change the schema. Any thoughts or other ideas?

[lucas-flexcompute]

ok, any possible unintended consequences though? For example a monitor_data.hdf5 from 2.4.1 won't be able to be loaded in 2.4.2? my vote is probably to wait for 2.5 unless you're pretty confident it will be fine.

@tylerflex, would it work if we defined an alias for n_group_raw? From my understanding, pydantic should use the alias for (de)serialization:

n_group_raw: ModeIndexDataArray = pd.Field(
    None,
    alias="n_group",
    title="Group Index",
    description="Index associated with group velocity of the mode.",
)

I don't think the alias would interfere with the actual field and property, right?

[tylerflex]

I don't think the alias would interfere with the actual field and property, right?

I was thinking of something similar (basically define a property getter for n_group on top of the field). Alias might work similarly. Unfortunately, I simply don't know how this would work, we'd have to try it out. I know MC used some aliases in the webapi and they seem to behave a bit strangely in my experience (or maybe I dont understand fully how they work) so I'm a little reluctant.

Throwing out another idea, there's a notion of a default_factory() in pydantic, which is basically defining your default value as a function. So we could do something like

def warn_n_group():
    log.warning(...)
    return None

  n_group: ModeIndexDataArray = pd.Field(
         default_factory = warn_n_group,
         ...
)

could be one thing to try maybe?

[tylerflex]

Eh, turns out the default factory runs on init time, not when the field is accessed, so that probably doesn't work. Was basically wondering if the function could have a default value as the return of a function that logs the warning, rather than just None.

[lucas-flexcompute]

Setting the alias option does indeed work. I'm running this branch on prod without errors.

However, once the server is updated, it'll send back data with "n_group_raw" by default, so users with previous versions will get an error. We'd have to set the serialization method for mode solver data to use the argument by_alias=True to output "n_group" instead if we want to have the 2.5 solver work with previous versions of the front-end. My understanding, though, is that we keep the solvers versioned and older front-end versions will run on their corresponding solver, won't they? So there isn't really any problem here.

[momchil-flex]

Just to make sure I understand what you're saying we will make 2.5 be able to read in both data that has n_group_raw (created by version >= 2.5) and data that has n_group (created by < 2.5), so 2.5 will be able to read in new and old data. Versions < 2.5 will not be able to read in data created by >= 2.5, but that's OK.

[lucas-flexcompute]

Just to make sure I understand what you're saying we will make 2.5 be able to read in both data that has n_group_raw (created by version >= 2.5) and data that has n_group (created by < 2.5), so 2.5 will be able to read in new and old data. Versions < 2.5 will not be able to read in data created by >= 2.5, but that's OK.

Correct mode solver data created with 2.5 won't be loadable in previous versions, data from previous versions can be loaded in 2.5