fix: address review comments, especially about setting up metric unit classes

Author: ilana-n

aiperf/common/enums/metric_enums.py

@@ -198,19 +198,97 @@ class GenericMetricUnit(BaseMetricUnit):
     PERCENT = _unit("%")
 
 
+class PowerMetricUnitInfo(BaseMetricUnitInfo):
+    """Information about a power unit for metrics."""
+
+    long_name: str
+    watts: float
+
+    def convert_to(self, other_unit: "MetricUnitT", value: int | float) -> float:
+        """Convert a value from this unit to another unit."""
+        if not isinstance(other_unit, PowerMetricUnit | PowerMetricUnitInfo):
+            return super().convert_to(other_unit, value)
+
+        return value * (self.watts / other_unit.watts)
+
+
 class PowerMetricUnit(BaseMetricUnit):
     """Defines power units for metrics."""
 
-    WATT = _unit("W")
-    MILLIWATT = _unit("mW")
+    WATT = PowerMetricUnitInfo(
+        tag="W",
+        long_name="watts",
+        watts=1.0,
+    )
+    MILLIWATT = PowerMetricUnitInfo(
+        tag="mW",
+        long_name="milliwatts",
+        watts=0.001,
+    )
+
+    @cached_property
+    def info(self) -> PowerMetricUnitInfo:
+        """Get the info for the power unit."""
+        return self._info  # type: ignore
+
+    @cached_property
+    def watts(self) -> float:
+        """The number of watts in the power unit."""
+        return self.info.watts
+
+    @cached_property
+    def long_name(self) -> str:
+        """The long name of the power unit."""
+        return self.info.long_name
+
+
+class EnergyMetricUnitInfo(BaseMetricUnitInfo):
+    """Information about an energy unit for metrics."""
+
+    long_name: str
+    joules: float
+
+    def convert_to(self, other_unit: "MetricUnitT", value: int | float) -> float:
+        """Convert a value from this unit to another unit."""
+        if not isinstance(other_unit, EnergyMetricUnit | EnergyMetricUnitInfo):
+            return super().convert_to(other_unit, value)
+
+        return value * (self.joules / other_unit.joules)
 
 
 class EnergyMetricUnit(BaseMetricUnit):
     """Defines energy units for metrics."""
 
-    JOULE = _unit("J")
-    MILLIJOULE = _unit("mJ")
-    MEGAJOULE = _unit("MJ")
+    JOULE = EnergyMetricUnitInfo(
+        tag="J",
+        long_name="joules",
+        joules=1.0,
+    )
+    MILLIJOULE = EnergyMetricUnitInfo(
+        tag="mJ",
+        long_name="millijoules",
+        joules=0.001,
+    )
+    MEGAJOULE = EnergyMetricUnitInfo(
+        tag="MJ",
+        long_name="megajoules",
+        joules=1_000_000.0,
+    )
+
+    @cached_property
+    def info(self) -> EnergyMetricUnitInfo:
+        """Get the info for the energy unit."""
+        return self._info  # type: ignore
+
+    @cached_property
+    def joules(self) -> float:
+        """The number of joules in the energy unit."""
+        return self.info.joules
+
+    @cached_property
+    def long_name(self) -> str:
+        """The long name of the energy unit."""
+        return self.info.long_name
 
 
 class MetricDateTimeUnit(BaseMetricUnit):

aiperf/common/enums/service_enums.py

@@ -46,7 +46,7 @@ class ServiceType(CaseInsensitiveStrEnum):
     WORKER = "worker"
     TELEMETRY_MANAGER = "telemetry_manager"
 
-    # [AIP-331] TODO: Remove when CLI argument is added to AI perf (–server-metrics-url) and telemetry_data_collector.py no longer requires local testing via main().
+    # [AIP-331] TODO: @ilana-n Remove when CLI argument is added to AI perf (–server-metrics-url) and telemetry_data_collector.py no longer requires local testing via main().
     TELEMETRY_COLLECTOR = "telemetry_collector"
 
     # For testing purposes only

aiperf/common/models/telemetry_models.py

@@ -18,7 +18,7 @@ class TelemetryRecord(AIPerfBaseModel):
     """
 
     timestamp_ns: int = Field(
-        description="Nanosecond timestamp when telemetry was collected (perf_counter_ns)"
+        description="Nanosecond wall-clock timestamp when telemetry was collected (time_ns)"
     )
 
     dcgm_url: str = Field(

aiperf/gpu_telemetry/telemetry_data_collector.py

@@ -155,7 +155,7 @@ def _parse_metrics_to_records(self, metrics_data: str) -> list[TelemetryRecord]:
             self._logger.warning("Response from DCGM metrics endpoint is empty")
             return []
 
-        current_timestamp = time.perf_counter_ns()
+        current_timestamp = time.time_ns()
         gpu_data = {}
         gpu_metadata = {}
 

aiperf/gpu_telemetry/telemetry_manager.py

@@ -62,7 +62,7 @@ def __init__(
 
         self._collectors: dict[str, TelemetryDataCollector] = {}
 
-        # TODO: Get from user_config when telemetry config is added
+        # [AIP-280] TODO: @ilana-n Get from user_config when telemetry config is added to CLI argument
         self._dcgm_endpoints = [DEFAULT_DCGM_ENDPOINT]
         self._collection_interval = DEFAULT_COLLECTION_INTERVAL