Skip to content

Conversation

@au650680
Copy link
Contributor

Clustering and mode tracking is added with example code for both and a shared module for both. Other small changes have been made.

au650680 added 4 commits October 21, 2025 12:46
Clustering and mode tracking is added with example code for both and a shared module for both. Other small changes have been made.
Clustering and mode tracking is added with example code for both and a shared module for both. Other small changes have been made.
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds clustering and mode tracking functionality to the system identification pipeline, with significant refactoring of existing code. The changes include renaming the main system ID module from sys_id to sysid_module, implementing new clustering and mode tracking algorithms, consolidating constants into a centralized PARAMS dictionary, and adding visualization capabilities.

Key changes:

  • Added new clustering algorithm (clustering.py) and mode tracking algorithm (mode_tracking.py) with supporting modules
  • Refactored constants from individual variables to a unified PARAMS dictionary structure
  • Removed old mode tracking implementation (mode_track.py) in favor of new approach

Reviewed Changes

Copilot reviewed 19 out of 19 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
tests/unit/methods/test_sys_id_unit.py Updated import to use renamed sysid_module
tests/integration/methods/test_sys_id.py Updated imports and function calls to use renamed sysid_module
src/methods/sysid_module.py Updated constants imports, removed 'Lab' field, added ordmin parameter, improved wait messaging
src/methods/packages/pyoma/ssiWrapper.py Added NaN inference for minimum order, commented out stability criteria code
src/methods/packages/mode_tracking.py New file implementing cluster tracking across experiments
src/methods/packages/mode_track.py Removed old mode tracking implementation (944 lines deleted)
src/methods/packages/clustering.py New file implementing frequency clustering algorithm (1206 lines)
src/methods/model_update_module.py Removed mode tracking functionality, updated imports
src/methods/constants.py Replaced individual constants with unified PARAMS dictionary structure
src/methods/clustering_tracking_module.py New module for clustering/tracking with MQTT subscription support
src/functions/sysid_plot.py New file with stabilization diagram and cluster plotting functions
src/functions/plot_mode_tracking.py New file for tracking visualization
src/examples/updating_parameters.py Updated to use new clustering instead of old mode tracking
src/examples/run_pyoma.py Updated imports, added looping functionality, improved wait messaging
src/examples/mode_tracking.py Refactored to use new clustering and tracking APIs
src/examples/example.py Added new CLI commands for clustering and tracking
src/examples/clustering.py New example file demonstrating clustering functionality
src/data/accel/metadata.py Added debug print for metadata payload
src/data/accel/hbk/aligner.py Added newline prefix to aligned shape print statement

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines 42 to 48
fig_ax = plot_clusters(dictionary_of_clusters, oma_output, PARAMS, fig_ax = None)
plt.show(block=False)
sys.stdout.flush()

def run_clustering_with_remote_sysid(config_path):
oma_output, dictionary_of_clusters = MT.subscribe_and_cluster(config_path,PARAMS)
fig_ax = plot_clusters(dictionary_of_clusters, oma_output, PARAMS, fig_ax = None)
Copy link

Copilot AI Oct 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fig_ax variable is assigned but never used. Either remove the assignment or pass it to subsequent plotting calls if incremental updates are needed.

Suggested change
fig_ax = plot_clusters(dictionary_of_clusters, oma_output, PARAMS, fig_ax = None)
plt.show(block=False)
sys.stdout.flush()
def run_clustering_with_remote_sysid(config_path):
oma_output, dictionary_of_clusters = MT.subscribe_and_cluster(config_path,PARAMS)
fig_ax = plot_clusters(dictionary_of_clusters, oma_output, PARAMS, fig_ax = None)
plot_clusters(dictionary_of_clusters, oma_output, PARAMS, fig_ax = None)
plt.show(block=False)
sys.stdout.flush()
def run_clustering_with_remote_sysid(config_path):
oma_output, dictionary_of_clusters = MT.subscribe_and_cluster(config_path,PARAMS)
plot_clusters(dictionary_of_clusters, oma_output, PARAMS, fig_ax = None)

Copilot uses AI. Check for mistakes.
def _on_metadata(client: MQTTClient, userdata, message) -> None:
try:
payload = json.loads(message.payload.decode("utf-8"))
print("Metadata",payload)
Copy link

Copilot AI Oct 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Debug print statement should use proper logging instead of print, or be removed for production code. Consider using logging.debug(f'Metadata: {payload}') instead.

Copilot uses AI. Check for mistakes.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@au650680 please remove the print statement

Copy link
Contributor

@prasadtalasila prasadtalasila left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@au650680 Jakob, thanks for the PR. I have made some comments on the code changes. Please let me know if something is not clear.


@cli.command()
@click.pass_context
def oma_and_publish_looping(ctx):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can this function be renamed to live_oma_and_publish?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes


def subscribe_and_get_clusters(config_path: str) -> Tuple[List[Dict], np.ndarray, np.ndarray]:
"""
Subscribes to MQTT broker, receives one OMA message, runs mode tracking, and returns results.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

function description here is same as that of subscribe_and_clusters. Is it possible to add a little more description that is suitable for each function.

sc["err_xi"],
sc["err_phi"],
)
# # Get the labels of the poles
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove these commented lines

time.sleep(0.1)
t2 = time.time()
t_text = f"Waiting for data for {round(t2-t1,1)} seconds"
print(t_text,end="\r")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few suggestions.

  1. please rename the file to plot_sysid.py
  2. There is a lot of repetition in matplotlib code. Is it possible to create small functions of matplotlib code that gets reused?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The packages directory is meant for putting code related from third-party packages like pyoma and yafem. Please move the clustering.py and mode_tracking.py to methods directory.
In addition, this is a HUGE file with multiple functions running into 100+ lines of code. Can you please split this into two or three modules (files) each containing smaller functions? Perhaps methods/clustering directory can contain these smaller modules containing smaller functions.

Copy link
Contributor Author

@au650680 au650680 Oct 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Okay, I just figured that since model_update.py and mode_pairs.py was already there, then similar files should be there too.
  2. I will look into it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The suggestions provided for clustering.py are valid here as well.

Splitting function blocks into smaller functions. Other small changes.
@prasadtalasila
Copy link
Contributor

@au650680 the tests are failing. Please try

pytest

on your computer to see the failing tests.

@au650680
Copy link
Contributor Author

au650680 commented Oct 30, 2025

@prasadtalasila, the pytest fails, since I have changed the naming of some functions from oma to sysid. The other test becuase the mqtt.eclipseprojects.io server is down.

au650680 added 3 commits October 31, 2025 09:29
Fixed an error in create_cluster, that resulted in doublicate model orders for a cluster. Allowed for scatter point size to be adjusted.
Removed debug info from some files, added missing type info for function arguments and returns, updated to YaFEM 1.0.0, changed some function descriptions, added timestamp to clustering data
@prasadtalasila
Copy link
Contributor

@au650680 I am still experiencing issues with one example. Please see the log

python .\src\examples\example.py sysid-and-publish
JSON configuration loaded successfully.
on_connect: Connected with response code Success
Subscribing to topic: cpsens/+/3053-B-120_sn_105283/1/acc/raw/metadata
on_subscribe: Subscription ID 1 with QoS levels [ReasonCode(Suback, 'Granted QoS 0')]
Extracted Fs from metadata: 256.0
Extracted FS from metadata: 256.0
JSON configuration loaded successfully.
Failed to extract FS from metadata. Using DEFAULT_FS.
on_connect: Connected with response code Success
Subscribing to topic: cpsens/+/3053-B-120_sn_105283/1/acc/raw/data
on_subscribe: Subscription ID 1 with QoS levels [ReasonCode(Suback, 'Granted QoS 1')]
on_connect: Connected with response code Not authorized
Connection failed with result code: Not authorized
on_subscribe: Subscription ID 2 with QoS levels [ReasonCode(Suback, 'Granted QoS 1')]
on_subscribe: Subscription ID 3 with QoS levels [ReasonCode(Suback, 'Granted QoS 1')]
on_subscribe: Subscription ID 4 with QoS levels [ReasonCode(Suback, 'Granted QoS 1')]
on_subscribe: Subscription ID 5 with QoS levels [ReasonCode(Suback, 'Granted QoS 1')]
on_subscribe: Subscription ID 6 with QoS levels [ReasonCode(Suback, 'Granted QoS 1')]
on_subscribe: Subscription ID 7 with QoS levels [ReasonCode(Suback, 'Granted QoS 1')]
on_subscribe: Subscription ID 8 with QoS levels [ReasonCode(Suback, 'Granted QoS 1')]
on_connect: Connected with response code Not authorized
Connection failed with result code: Not authorized
on_connect: Connected with response code Not authorized
Connection failed with result code: Not authorized
on_connect: Connected with response code Not authorized
Connection failed with result code: Not authorized
Waiting for data for 12.1 seconds
Aligned shape: (4, 3072)
Data dimensions: (3072, 4)
sysid parameters: {'freq_variance_treshold': 0.1, 'damp_variance_treshold': 1000000, 'Fs': 256, 'model_order_min': 2, 'model_order': 15, 'block_shift': 30, 'sensor_order': array([0, 2, 1, 3]), 'mstab': 6, 'tMAC': 0.95, 'bound_multiplier': 2, 'allignment_factor': [0.05, 0.01], 'phi_cri': 0.8, 'freq_cri': 0.2, 'obj_cri': 0.1}
2025-11-05 17:39:29,167 - pyoma2.setup.base - INFO - Running SSIcovmm_mt... (base:123)
2025-11-05 17:39:29,168 - pyoma2.functions.ssi - INFO - Assembling Hankel matrix method: cov_mm... (ssi:82)
2025-11-05 17:39:29,172 - pyoma2.functions.ssi - INFO - ... uncertainty calculations... (ssi:94)
2025-11-05 17:39:29,211 - pyoma2.functions.ssi - INFO - SSI for increasing model order... (ssi:359)
100%|███████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████| 16/16 [00:00<00:00, 11305.40it/s]
2025-11-05 17:39:29,258 - pyoma2.functions.ssi - INFO - Calculating uncertainty... (ssi:367)
 60%|█████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████                                                                                      | 9/15 [00:02<00:01,  3.87it/s]on_connect: Connected with response code Not authorized
Connection failed with result code: Not authorized
100%|██████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████| 15/15 [00:04<00:00,  3.47it/s]
2025-11-05 17:39:33,579 - pyoma2.functions.ssi - INFO - Calculating modal parameters... (ssi:531)
100%|█████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████| 15/15 [00:00<00:00, 152.36it/s]
Timestamp: 2025-11-05 17:39:29.160016
Publisher disconnected. Reconnecting...
[2025-11-05T17:39:29.160016] Published sysid result to cpsens/d8-3a-dd-37-d2-7e/3050-A-060_sn_106209/1/acc/sysid/data
Publishing to topic: cpsens/d8-3a-dd-37-d2-7e/3050-A-060_sn_106209/1/acc/sysid/data

@prasadtalasila
Copy link
Contributor

Also please delete poetry.lock file, perform poetry install. It will generate a new poetry.lock file. Please commit this file.

@prasadtalasila prasadtalasila merged commit 66e00b8 into INTO-CPS-Association:main Nov 9, 2025
0 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants