feat(medium): track parent medium from which the medium is derived

[weiliangjin2021]

Not elegant, especially the quote in the type.

Greptile Overview

Greptile Summary

Added a super_medium field to AbstractMedium that tracks the parent medium from which a derived medium was created. This is set in the perturbed_copy() methods of PerturbationMedium and PerturbationPoleResidue, allowing users to trace back to the original perturbation medium.

Key Changes:

• Added super_medium: Optional["MediumType"] field to AbstractMedium with forward reference
• Updated AbstractMedium.update_forward_refs() call (though it currently resolves to Any instead of the full MediumType union)
• Modified perturbed_copy() in both PerturbationMedium and PerturbationPoleResidue to set super_medium = self before returning derived mediums
• Added comprehensive test coverage for the new field

Issues Found:

• Forward reference resolution uses Any instead of the actual MediumType, which reduces type safety
• Missing CHANGELOG entry for this user-facing feature

Confidence Score: 4/5

• This PR is generally safe to merge with one logical issue that should be addressed
• The implementation correctly adds the super_medium field and sets it appropriately in all code paths. The test coverage is excellent and validates the expected behavior. However, the forward reference resolution at line 1166 uses Any instead of the full MediumType union, which undermines type safety for the new field. Additionally, a CHANGELOG entry is missing per project guidelines.
• Pay close attention to tidy3d/components/medium.py line 1166 for the forward reference resolution issue

Important Files Changed

File Analysis

Filename	Score	Overview
tidy3d/components/medium.py	4/5	Added super_medium field to track parent medium and updated forward reference resolution; needs CHANGELOG entry
tests/test_components/test_perturbation_medium.py	5/5	Comprehensive test coverage for super_medium field on both PerturbationMedium and PerturbationPoleResidue

Sequence Diagram

sequenceDiagram
    participant User
    participant PerturbationMedium
    participant Medium
    participant CustomMedium
    
    User->>PerturbationMedium: perturbed_copy()
    alt No perturbation data
        PerturbationMedium->>PerturbationMedium: Create new_dict excluding perturbation fields
        PerturbationMedium->>PerturbationMedium: Set super_medium = self
        PerturbationMedium->>Medium: parse_obj(new_dict)
        Medium-->>User: Return Medium with super_medium set
    else With perturbation data
        PerturbationMedium->>PerturbationMedium: Apply perturbations to fields
        PerturbationMedium->>PerturbationMedium: Set super_medium = self
        PerturbationMedium->>CustomMedium: parse_obj(new_dict)
        CustomMedium-->>User: Return CustomMedium with super_medium set
    end

Loading

Context used:

• Rule from dashboard - Require a changelog entry for any PR that is not purely an internal refactor. (source)

[greptile-apps]

_{2 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

[github-actions]

Diff Coverage

Diff: origin/develop...HEAD, staged and unstaged changes

• tidy3d/components/medium.py (100%)
• tidy3d/components/simulation.py (76.9%): Missing lines 2246,2249-2250
• tidy3d/components/structure.py (71.4%): Missing lines 264,269

Summary

• Total: 36 lines
• Missing: 5 lines
• Coverage: 86%

tidy3d/components/simulation.py

  2242         -------
  2243         Dict[:class:`.AbstractMedium`, int]
  2244             Mapping between distinct mediums to index in finalized simulation.
  2245         """
! 2246         medium_set = {
  2247             structure._optical_medium for structure in self._finalized_volumetric_structures
  2248         }
! 2249         medium_set.add(Structure._get_optical_medium(self.medium))
! 2250         return {medium: index for index, medium in enumerate(medium_set)}
  2251 
  2252     def _validate_finalized(self) -> None:
  2253         """Validate that after adding pec frames simulation setup is still valid."""

---

tidy3d/components/structure.py

  260 
  261     @staticmethod
  262     def _get_optical_medium(medium):
  263         """Get optical medium."""
! 264         return medium.optical if isinstance(medium, MultiPhysicsMedium) else medium
  265 
  266     @property
  267     def _optical_medium(self) -> StructureMediumType:
  268         """Optical medium of the structure."""
! 269         return self._get_optical_medium(self.medium)
  270 
  271     @pydantic.validator("medium", always=True)
  272     @skip_if_fields_missing(["geometry"])
  273     def _check_2d_geometry(cls, val, values):

---

[weiliangjin2021]

Adding the class itself as its field looks messy with pydantic. Probably an identifier field is a cleaner implementation, unless @momchil-flex you have better idea to improve the current PR.

[dbochkov-flexcompute]

Would it help if we add the super_medium (or parent_medium) only to AbstractCustomMedium? I don't think there is a case when we end up in the same perturbed simulation with two different mediums, one Medium and one CustomMedium, derived from the same PerturbationMedium. They should either be all Medium's (if no perturbation fields are passed) or all of them are CustomMedium's. This should also allow us to easily add CustomMedium.perturbed_copy for resampling on a different set perturbation fields

[weiliangjin2021]

Would it help if we add the super_medium (or parent_medium) only to AbstractCustomMedium? I don't think there is a case when we end up in the same perturbed simulation with two different mediums, one Medium and one CustomMedium, derived from the same PerturbationMedium. They should either be all Medium's (if no perturbation fields are passed) or all of them are CustomMedium's. This should also allow us to easily add CustomMedium.perturbed_copy for resampling on a different set perturbation fields

That's right. On the other hand, we'll also want to keep track when there is no perturbation, so that the two generated materials are still considered the same. Then with perturbation applied, the subpixel should be applied in the same way to avoid abrupt change.

[dbochkov-flexcompute]

Would it help if we add the super_medium (or parent_medium) only to AbstractCustomMedium? I don't think there is a case when we end up in the same perturbed simulation with two different mediums, one Medium and one CustomMedium, derived from the same PerturbationMedium. They should either be all Medium's (if no perturbation fields are passed) or all of them are CustomMedium's. This should also allow us to easily add CustomMedium.perturbed_copy for resampling on a different set perturbation fields

That's right. On the other hand, we'll also want to keep track when there is no perturbation, so that the two generated materials are still considered the same. Then with perturbation applied, the subpixel should be applied in the same way to avoid abrupt change.

actually, if only AbstractCustomMedium carries parent_medium, then if you do psim = sim.perturbation_mediums_copy(temperature=None, electron_density=None, hole_density=None), then there would be no way to apply perturbations to psim. But maybe we should change AbstractPerturbationMedium.perturbed_copy() to return itself if no perturbation are provided. This should resolve both issues.

[weiliangjin2021]

actually, if only AbstractCustomMedium carries parent_medium, then if you do psim = sim.perturbation_mediums_copy(temperature=None, electron_density=None, hole_density=None), then there would be no way to apply perturbations to psim. But maybe we should change AbstractPerturbationMedium.perturbed_copy() to return itself if no perturbation are provided. This should resolve both issues.

That's a good idea!