Currently, the Tokamak class serves as a common interface for various devices, and it loads data lazily based on availability. There are two things to consider, and they're orthogonal:
- The
Tokamak class contains a number of different ways to extract the same information, and uses fallbacks to find data elsewhere if it's unsuccessful. That's fine in principle, but it can lead to unintended behavior changes if the underlying data changes.
Suggestion: derive "device description" files and commit them, so that this process becomes more deliberate. This is a mild suggestion, and whether it's appropriate for Emis3D depends in part also on how static all this data is.
- The
Tokamak class doesn't provide a standardized interface/contract for Emis3D to work with. It has several attributes that are created dynamically (e.g., info, cameras), and filled lazily. It provides no guarantees to the consumers as to what's contained in these attributes, or whether they exist in the first place. That puts a burden on consumers to keep re-validating the config.
Suggestion: turn Tokamak into pydantic class (or several nested classes), and do validation early, so that consumers don't need to guess what is in there, and static analyzers can find mistakes early.
Example of what config classes might look like, extracted from Emis3D code:
class Location(BaseModel):
"""A point in the machine's cylindrical frame (R, z in meters; toroidal angle in degrees)."""
r_m: float
z_m: float
phi_deg: float
class Channel(BaseModel):
"""One detector channel: its label and its physical offset along the foil array."""
tag: str
foil_offset_m: float # distance from the sensor center
class Diagnostic(BaseModel):
"""A pinhole radiation camera — where it sits, where it looks, and its detector optics."""
name: str
group: str
position: Location
rotation_deg: float
downward_facing: bool
skew_deg: float = 0.0
# pinhole + foil-array optics
slit_width_m: float
slit_height_m: float
foil_width_m: float
foil_height_m: float
foil_corner_curvature_m: float
slit_sensor_separation_m: float
channels: list[Channel]
class FirstWall(BaseModel):
"""The poloidal limiting contour the plasma sees, as (R, z) points in meters."""
rz_m: list[tuple[float, float]]
class CadModel(BaseModel):
"""A 3-D plasma-facing-component mesh for rendering and sightline occlusion, loaded on demand."""
model_config = ConfigDict(arbitrary_types_allowed=True)
units: Literal["m", "mm"] = "m"
rotate_z_deg: float = 0.0
_source: Path = PrivateAttr() # location is an internal detail, never exposed to consumers
@cached_property
def mesh(self):
"""Read the STL at _source, scale by units, rotate about z — loaded once, then cached."""
... # the only place I/O happens; consumers see geometry, never a path
class Tokamak(BaseModel):
"""The static description of a device — everything true across all shots."""
name: str
major_radius_m: float
minor_radius_m: float
volume_m3: float
phi_offset_to_emis3d_rad: float # rotates this machine's toroidal angles into the Emis3D frame
first_wall: FirstWall
diagnostics: list[Diagnostic] = []
cad: CadModel | None = None # optional; only needed for high-fidelity rendering
Currently, the
Tokamakclass serves as a common interface for various devices, and it loads data lazily based on availability. There are two things to consider, and they're orthogonal:Tokamakclass contains a number of different ways to extract the same information, and uses fallbacks to find data elsewhere if it's unsuccessful. That's fine in principle, but it can lead to unintended behavior changes if the underlying data changes.Suggestion: derive "device description" files and commit them, so that this process becomes more deliberate. This is a mild suggestion, and whether it's appropriate for Emis3D depends in part also on how static all this data is.
Tokamakclass doesn't provide a standardized interface/contract for Emis3D to work with. It has several attributes that are created dynamically (e.g.,info,cameras), and filled lazily. It provides no guarantees to the consumers as to what's contained in these attributes, or whether they exist in the first place. That puts a burden on consumers to keep re-validating the config.Suggestion: turn
Tokamakinto pydantic class (or several nested classes), and do validation early, so that consumers don't need to guess what is in there, and static analyzers can find mistakes early.Example of what config classes might look like, extracted from Emis3D code: