Skip to content

Conversation

@ncguilbeault
Copy link
Collaborator

@ncguilbeault ncguilbeault commented Sep 29, 2025

Summary

This PR introduces a new package, Bonsai.ML.Torch.LDS, which implements linear dynamical systems (LDS) using TorchSharp for online filtering, smoothing, and parameter estimation in Bonsai. This package differs from the Bonsai.ML.LinearDynamicalSystems package in that it does not depend on Python, and the models parameters can be customised. A test was added to compare the output of the TorchSharp implementation with the output of the lds_python package following parameter estimation and filtering of neural recordings.

The PR in #77 should be merged first.

Refactoring the unit test to download the expected results instead of generating them with a Python script fixes #75

@ncguilbeault ncguilbeault added the feature New planned feature label Sep 29, 2025
@ncguilbeault ncguilbeault force-pushed the dev/torch-lds branch 3 times, most recently from 6b0e89b to 93a8db0 Compare September 30, 2025 16:06
@glopesdev
Copy link
Member

glopesdev commented Oct 10, 2025

Leaving here a summary of what we discussed earlier about deprecating the Bonsai.ML.LinearDynamicalSystems package and adopting back-end suffixes for systems with different implementations. In this case the steps would be:

  • Rename the package to Bonsai.ML.Lds.Torch
  • Rename Bonsai.ML.LinearDynamicalSystems to Bonsai.ML.Lds.Python
  • Deprecate the Bonsai.ML.LinearDynamicalSystems package

The one consideration we discussed was whether to "nest" namespaces, e.g. we could use the name Bonsai.ML.Torch.Lds to make it so that all types in Bonsai.ML.Torch would be automatically visible to ML packages using the Torch backend. In the end we decided to go with the suffix, and import functionality where needed, similar to what is done with most non-overlapping .NET namespaces.

@ncguilbeault
Copy link
Collaborator Author

Updating the namespace of the existing Bonsai.ML.LinearDynamicalSystems resulted in quite a few changes. Because of this, I decided to push these changes to a separate PR which we should merge first before this one. The new PR is here: #74

Copy link
Member

@glopesdev glopesdev left a comment

Choose a reason for hiding this comment

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

Looks good, left only a few minor comments to review.

docs/README.md Outdated
> [!NOTE]
> Bonsai.ML packages can be installed through Bonsai's integrated package manager and are generally ready for immediate use. However, some packages may require additional installation steps. Refer to the specific package section for detailed installation guides and documentation.
### Bonsai.ML.Torch.LDS
Copy link
Member

Choose a reason for hiding this comment

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

Rename to match package and project name.

@@ -0,0 +1,7 @@
# Bonsai.ML.Torch.LDS - Overview
Copy link
Member

Choose a reason for hiding this comment

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

Same as above.


## Installation Guide

Install the `Bonsai.ML.Torch.LDS` package from the Bonsai package manager. You will also need to follow the [instructions for setting up the Bonsai.ML.Torch package](../Torch/torch-overview.md) for running on the CPU or GPU. No newline at end of file
Copy link
Member

Choose a reason for hiding this comment

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

Rename the package name as above.

- name: Overview
href: Torch/torch-overview.md
href: Torch/torch-overview.md
- name: Torch.LDS
Copy link
Member

Choose a reason for hiding this comment

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

Same as above to align the naming.


using static TorchSharp.torch;

[assembly: TypeVisualizer(typeof(Bonsai.ML.Lds.Torch.Design.ExpectationMaximizationVisualizer),
Copy link
Member

Choose a reason for hiding this comment

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

Just a thought but given the current decision to lean more on acronyms, this could also be called EMVisualizer.

However, we do have a class already that is named ExpectationMaximization and we probably don't want to contract that to EM, and I do also like the verbose name as it is, so I am torn and more than happy to go either way and leave it as-is.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I suggest we keep the full name for now and we can revisit down the line if we want to change it

/// <summary>
/// Represents the state of a linear gaussian dynamical system.
/// </summary>
public interface ILdsState
Copy link
Member

Choose a reason for hiding this comment

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

I have been wondering whether we should expand Lds inside class names (i.e. leave only the package name with the acronym, but expand inner names).

Just leaving this comment here as the name of this interface was quite opaque to decipher, versus the alternative ILinearDynamicalSystemState.

/// </summary>
/// <param name="smoothedMean"></param>
/// <param name="smoothedCovariance"></param>
public struct SmoothedState(
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason we need this specific type? Looks like we might be able to simply use the LdsState class to represent the smoothed state.

[ResetCombinator]
[Description("Updates the parameters of a Kalman filter model instance using the provided Kalman filter parameters.")]
[WorkflowElementCategory(ElementCategory.Sink)]
public class UpdateParameters
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
public class UpdateParameters
public class UpdateKalmanFilterParameters

Should this be renamed for consistency with the Save/Load methods?

@ncguilbeault ncguilbeault force-pushed the dev/torch-lds branch 3 times, most recently from d041405 to f857e16 Compare November 11, 2025 14:29
@ncguilbeault ncguilbeault force-pushed the dev/torch-lds branch 4 times, most recently from eeb66f6 to a237e2c Compare December 16, 2025 11:24
…osed to running both the Python and Bonsai code to save memory
…Observations` and `numStates` are not provided
…her than individual files which is more consistent with other `Bonsai.ML` packages
…ved validation logic outside of `KalmanFilter` module
…including moving devices, setting scalar types, and setting gradient tracking
…be loaded without explicitly setting type and fixed path support
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature New planned feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

GitHub Action Runner Failing Due to Out of Disk Space Exception During Unit Test Execution

2 participants