-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve documentation of IncludeWorkflows #40
base: main
Are you sure you want to change the base?
Conversation
.bonsai/Bonsai.config
Outdated
<Package id="Bonsai.ML" version="0.4.0" /> | ||
<Package id="Bonsai.ML.Data" version="0.4.0" /> | ||
<Package id="Bonsai.ML.Design" version="0.4.0" /> | ||
<Package id="Bonsai.ML.HiddenMarkovModels" version="0.4.0" /> | ||
<Package id="Bonsai.ML.HiddenMarkovModels.Design" version="0.4.0" /> | ||
<Package id="Bonsai.ML.LinearDynamicalSystems" version="0.4.0" /> | ||
<Package id="Bonsai.ML.LinearDynamicalSystems.Design" version="0.4.0" /> | ||
<Package id="Bonsai.ML.NeuralDecoder" version="0.4.0" /> | ||
<Package id="Bonsai.ML.NeuralDecoder.Design" version="0.4.0" /> | ||
<Package id="Bonsai.ML.Python" version="0.4.0" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the source for these packages is already in this repo, I feel we might not want to add them to the documentation environment, and instead simply use the local builds.
The advantage of the latter is that we don't need to remember to keep updating the versions in the config file, since it will always be using what exactly is in the repository source code.
@@ -6,10 +6,11 @@ | |||
xmlns:p2="clr-namespace:Bonsai.ML.HiddenMarkovModels;assembly=Bonsai.ML.HiddenMarkovModels" | |||
xmlns:py="clr-namespace:Bonsai.Scripting.Python;assembly=Bonsai.Scripting.Python" | |||
xmlns="https://bonsai-rx.org/2018/workflow"> | |||
<Description>Creates a new HMM python object inside of the HMM module and assigns it to a named python variable.</Description> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is more a general question about casing in documentation. Do we want to use python
or Python
? I think I am happy one way or the other as long as we are consistent. I may have a slight preference to Python
since that is how it is cased in c# namespaces and classes, e.g. Python.NET
, Bonsai.Scripting.Python
, etc.
<Property Name="NumStates" Description="The number of discrete latent states of the HMM model." /> | ||
<Property Name="Dimensions" Description="The dimensionality of the observations into the HMM model." /> | ||
<Property Name="ObservationsModelType" Description="The type of distribution that the HMM will use to model the emission of data observations." /> | ||
<Property Name="StateParameters" Description="The optional state parameters of the HMM model. If an observations model or transitions model is defined within the state parameters, they will override the values set in the observations model type and transitions model type properties, respectively." /> | ||
<Property Name="TransitionsModelType" Description="The type of transition model that the HMM will use to calculate the probabilities of transitioning between states."/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<Property Name="NumStates" Description="The number of discrete latent states of the HMM model." /> | |
<Property Name="Dimensions" Description="The dimensionality of the observations into the HMM model." /> | |
<Property Name="ObservationsModelType" Description="The type of distribution that the HMM will use to model the emission of data observations." /> | |
<Property Name="StateParameters" Description="The optional state parameters of the HMM model. If an observations model or transitions model is defined within the state parameters, they will override the values set in the observations model type and transitions model type properties, respectively." /> | |
<Property Name="TransitionsModelType" Description="The type of transition model that the HMM will use to calculate the probabilities of transitioning between states."/> | |
<Property Name="NumStates" Description="The number of discrete latent states in the HMM." /> | |
<Property Name="Dimensions" Description="The dimensionality of the observations going into the HMM." /> | |
<Property Name="ObservationsModelType" Description="The type of distribution that the HMM will use to model the emission of data observations." /> | |
<Property Name="StateParameters" Description="The optional state parameters of the HMM. If an observations model or transitions model is defined within the state parameters, these will override the values set in the observations model type and transitions model type properties, respectively." /> | |
<Property Name="TransitionModelType" Description="The type of model that the HMM will use to calculate the probabilities of transitioning between states."/> |
A few suggestions to improve clarity and simplify sentence structure. Also in general I think we don't need the word "model" after HMM since it is included in the acronym.
A final question about the property TransitionsModelType
. In general one rule of thumb is to try and avoid pluralizing words unless we are talking about manipulation of collections, so I would rather use TransitionModelType
, but we can definitely discuss further.
@@ -64,39 +65,39 @@ | |||
</Combinator> | |||
</Expression> | |||
<Expression xsi:type="ExternalizedMapping"> | |||
<Property Name="Value" DisplayName="NumIterations" Category="Hyperparameters" /> | |||
<Property Name="Value" DisplayName="NumIterations" Description="The number of iterations to use for fitting the model to a single batch of data." Category="Hyperparameters" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<Property Name="Value" DisplayName="NumIterations" Description="The number of iterations to use for fitting the model to a single batch of data." Category="Hyperparameters" /> | |
<Property Name="Value" DisplayName="NumIterations" Description="The number of iterations used for fitting the model to a single batch of data." Category="Hyperparameters" /> |
<Property Name="Fps" Description="The frames per second of the observations." Category="ModelParams" /> | ||
<Property Name="Pos_x0" Description="The initial x position." Category="ModelParams" /> | ||
<Property Name="Pos_y0" Description="The initial y position." Category="ModelParams" /> | ||
<Property Name="Vel_x0" Description="The initial x velocity." Category="ModelParams" /> | ||
<Property Name="Vel_y0" Description="The initial y velocity." Category="ModelParams" /> | ||
<Property Name="Acc_x0" Description="The initial x acceleration." Category="ModelParams" /> | ||
<Property Name="Acc_y0" Description="The initial y acceleration." Category="ModelParams" /> | ||
<Property Name="Sigma_a" Description="A scalar value representing the measurement noise." Category="ModelParams" /> | ||
<Property Name="Sigma_x" Description="A scalar value representing the prediction noise along the x axis." Category="ModelParams" /> | ||
<Property Name="Sigma_y" Description="A scalar value representing the prediction noise along the y axis." Category="ModelParams" /> | ||
<Property Name="Sqrt_diag_V0_value" Description="The initial value of the diagonal of the state covariance matrix." Category="ModelParams" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general in Bonsai / C# we would use Pascal casing rather than snake case, but no need to change anything now, this is just a general comment for us to think about regarding casing of properties coming from Python.
@@ -51,10 +51,10 @@ public class KFModelParameters | |||
private string fpsString; | |||
|
|||
/// <summary> | |||
/// x position at time 0 | |||
/// The initial x position. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Depending on casing we would change this to X
but see comment above, no need to change anything now.
@@ -6,6 +6,7 @@ | |||
xmlns:rx="clr-namespace:Bonsai.Reactive;assembly=Bonsai.Core" | |||
xmlns:scr="clr-namespace:Bonsai.Scripting.Expressions;assembly=Bonsai.Scripting.Expressions" | |||
xmlns="https://bonsai-rx.org/2018/workflow"> | |||
<Description>Periodically evaluates the `get_optimization_finished` function of the LDS model. Returns true when the `optimization_finished` attribute is true. This should onle be used after the `RunOptimizationAsync` function has already been started.</Description> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<Description>Periodically evaluates the `get_optimization_finished` function of the LDS model. Returns true when the `optimization_finished` attribute is true. This should onle be used after the `RunOptimizationAsync` function has already been started.</Description> | |
<Description>Periodically evaluates the `get_optimization_finished` function of the LDS model. Returns whether the `optimization_finished` attribute is true. This should only be used after the `RunOptimizationAsync` function has already been started.</Description> |
<Workflow> | ||
<Nodes> | ||
<Expression xsi:type="WorkflowInput"> | ||
<Name>Source1</Name> | ||
</Expression> | ||
<Expression xsi:type="ExternalizedMapping"> | ||
<Property Name="Value" DisplayName="BatchSize" Category="TrainingData" /> | ||
<Property Name="Value" DisplayName="BatchSize" Description="The size of the batch of data to use for fitting the model." Category="TrainingData" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<Property Name="Value" DisplayName="BatchSize" Description="The size of the batch of data to use for fitting the model." Category="TrainingData" /> | |
<Property Name="Value" DisplayName="BatchSize" Description="The size of the batch used for fitting the model." Category="TrainingData" /> |
4f7f5c3
to
dcf81d2
Compare
Summary
This PR updates the main bonsai environment with the latest Bonsai.ML packages and adds documentation to IncludeWorkflows within Bonsai.ML packages. Fixes #32