Repository navigation
[UPDATE] Replace output's name with id for data management - #12
Open
kavehshahedi wants to merge 1 commit into
Open
kavehshahedi wants to merge 1 commit into
kavehshahedi wants to merge 1 commit into
Conversation
kavehshahedi
force-pushed
the
replace-name-with-id
branch
from
August 27, 2026 23:37
3466b3b to
f9893cc
Compare
kavehshahedi
requested
a lite review from Copilot
and removed request for
bhufmann
August 27, 2026 23:37
There was a problem hiding this comment.
Pull request overview
This PR updates the ML modules’ internal dataframe management to key and reference series by output ID instead of output name, addressing ambiguity when multiple outputs share the same name (e.g., multi-trace “Memory Usage” outputs).
Changes:
- Switched dataframe keys/column references across multiple modules from output name → output ID.
- Added
Experiment.get_output_by_id/get_output_by_name/get_outputs_by_namehelpers and introduced duplicate-name disambiguation inassign_outputs(). - Updated plotting/interpretation code to map IDs back to human-readable names for display.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tmll/ml/modules/root_cause/correlation_module.py | Correlation/lag analysis now indexes series by output ID and maps IDs back to names for plot labels. |
| tmll/ml/modules/resource_optimization/idle_resource_detection_module.py | Uses output IDs for stored dataframes and resolves IDs back to names when reporting/plotting. |
| tmll/ml/modules/predictive_maintenance/capacity_planning_module.py | Capacity planning results/plots now use output IDs as keys and map to names for display. |
| tmll/ml/modules/performance_trend/change_point_module.py | Resolves requested metrics to output IDs and maps IDs back to names in plots. |
| tmll/ml/modules/common/data_fetch.py | Renames fetched XY dataframe columns to output ID instead of output name. |
| tmll/ml/modules/base_module.py | Stores processed dataframes under output ID instead of output name. |
| tmll/ml/modules/anomaly_detection/memory_leak_detection_module.py | Refactors Events Table / Memory Usage access to use output IDs (and disambiguate Memory Usage). |
| tmll/ml/modules/anomaly_detection/anomaly_detection_module.py | Plotting now resolves dataframe keys (IDs) back to output names for labels. |
| tmll/common/models/experiment.py | Adds output lookup helpers and disambiguates duplicate output names during assignment. |
Suppressed comments (1)
tmll/ml/modules/anomaly_detection/memory_leak_detection_module.py:199
analyze_memory_leaks()now only checks forNonefrom_get_events_dataframe()/_get_memory_usage_dataframe(), but those helpers can return an empty DataFrame. In that case_analyze_memory_trend()returns{}andmemory_trend["p_value"]will raiseKeyError. This should treat empty DataFrames the same as missing data and return a NONE severity result.
et_df = self._get_events_dataframe()
if et_df is None:
self.logger.warning("Events table output not found in the experiment")
return LeakAnalysisResult(
severity=MemoryLeakSeverity.NONE,
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
As now, we use output's id instead of its name for a more robust and accurate data managements. Hence, for cases with multiple outputs with same names, the ids will now diffrenciate them. Signed-off-by: Kaveh Shahedi <kavehshahedi@gmail.com>
kavehshahedi
force-pushed
the
replace-name-with-id
branch
from
October 11, 2026 00:40
f9893cc to
962c8b8
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved lookup, resource classification, and plotting issues remain.
8 open findings
3 resolved since last review
🧠 Review effort: Lite
| df = self.data_preprocessor.remove_minimum(df) | ||
|
|
||
| self.dataframes[shortened] = df | ||
| self.dataframes[output.id] = df |
| if partial_match: | ||
| return [o for o in self.outputs if output_name.lower() in o.name.lower()] | ||
|
|
||
| return [o for o in self.outputs if o.name.lower() == output_name.lower()] |
| self.logger.warning("Events table does not contain necessary columns for memory leak analysis") | ||
| self.dataframes["Events Table"] = pd.DataFrame() | ||
| return | ||
| events_output = self.experiment.get_output_by_name("Events Table") |
|
|
||
| def _get_metric_data(metric_name: str) -> Tuple[str, Optional[pd.Series]]: | ||
| match metric_name: | ||
| def _get_metric_data(metric: str) -> Tuple[str, Optional[pd.Series]]: |
|
|
||
| forecast_values = self.scalers[name].inverse_transform(np.array(forecast_values) | ||
| .reshape(-1, 1)).flatten().tolist() | ||
| forecast_values = self.scalers[name].inverse_transform(np.array(forecast_values).reshape(-1, 1)).flatten().tolist() # type: ignore |
| overall_metrics = {} | ||
| for resource_type in ResourceType: | ||
| if not any(resource_type.name.lower() in name.lower() for name in self.dataframes): | ||
| if not any(resource_type.name.lower() in id.lower() for id in self.dataframes): |
| if series1_name not in self.dataframes or series2_name not in self.dataframes: | ||
| output1 = self.experiment.get_output_by_name(series1_name) | ||
| output2 = self.experiment.get_output_by_name(series2_name) | ||
| if output1 is None or output2 is None: |
| for idx, name in enumerate(series): | ||
| if name not in self.dataframes: | ||
| output = self.experiment.get_output_by_name(name) | ||
| if output is None: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What it does
Until now, we have utilized output names (e.g., "CPU Usage" or "Disk I/O View") to manage data frames inside the modules. Although this methodology seems to work fine, it becomes problematic for cases where we have multiple outputs with the same name. For instance, when fetching an LTTng trace (kernel + UST) from the trace server, we might get two "Memory Usage" outputs, one is for the kernel's memory usage and the other one comes from the UST.
As of now, we use the output's id for this matter, enabling us to accurately differentiate the outputs from each other as each output has its own unique id.
How to test
You can either run the tests using
pytest -v, or check the results/graphs/interpretations of each ML module (e.g., Anomaly Detection, Capacity Planning, etc.).Follow-ups
N/A
Review checklist