Fix bug in loading previously run Batch and ComponentModeler from file when custom medium present

[tylerflex]

Fixes #1816.

[momchil-flex]

We can get this in for now if it fixes things but it really looks like it needs a rethink... Two things strike me as not good design:

1. The existence and difference of both Job._cached_properties["task_id"] and a Job.task_id_cached and the fact that those differ, and the fact that we have to call updated_copy on Job just to update this.
2. self = self.updated_copy(batch_cached=batch_cached) just looks... bad, no?

[tylerflex]

Yea I'm also not very happy with it. Ultimately, it comes down to (a) our choice of immutability (b) making things like task_id properties.

For example, the Job.task_id doesn't exist until the Job is .upload()-ed. So it makes sense to make it a property. However, this task_id property gets lost when we save Job to file. So if the job has been run, after loading from file it seems like it has not. So either:

1. We manually save task_id to file, and load it as a special field (this PR)
2. We make Job and Batch mutable, and set self.task_id = task_id upon upload.
3. We make a more general way to handle saving and loading cached_property fields to and from file.

Any thoughts? In some sense, we treat Job and Batch as mutable objects, since they have a state (whether they've been updated, run, etc). In fact the task itself has a state too.. so this could make some sense to me.

[tylerflex]

Also, we never upload Job and Batch directly

The existence and difference of both Job._cached_properties["task_id"] and a Job.task_id_cached and the fact that those differ,

They have to differ because one is a property and one is a field.. in pydantic v2, there are computed_fields, which sort of are properties that can be saved to file, this is what we would need. For now we're sort of stuck with this.

and the fact that we have to call updated_copy on Job just to update this.

Yea, because of the immutability, I dont see a good way around this.

self = self.updated_copy(batch_cached=batch_cached) just looks... bad, no?

It sure does, but I dont see any way. Note that because of how I'm using super(SuperClass, new_self), I dont think re-assigning self matters here.

[momchil-flex]

Generally this seems to be pointing towards potentially making container classes mutable, which may not be a terrible idea? The contained simulations will still be immutable unless I'm really misunderstanding something.

[tylerflex]

Yea I think in general it would make some sense to create a distinction between (1) core components tidy3d/components, which can continue to be immutable and (2) web components tidy3d/web, which essentially transform these core components, eg Job(simulation).run() -> SimulationData, which could be mutable?

Although I think maybe this could go into the ominous web API refactor or the 3.0 redesign..

[momchil-flex]

Yea I think in general it would make some sense to create a distinction between (1) core components tidy3d/components, which can continue to be immutable and (2) web components tidy3d/web, which essentially transform these core components, eg Job(simulation).run() -> SimulationData, which could be mutable?

Sorry, which part here would be mutable? Just Job?

Although I think maybe this could go into the ominous web API refactor or the 3.0 redesign..

This wouldn't really be compatibility breaking in any user-facing way and may not be too big of a change so there may not be a need to wait for that, but I guess depends on priority if we have something working even if ugly.

[tylerflex]

just Job and Batch would be mutable.

guess we wouldn't need to wait, I'm just not sure if we want to open this can of worms just for this bug fix, and should consider waiting for a time when we're doing a more comprehensive overhaul.

[momchil-flex]

Ok merging this and made an issue to follow up. #1823