Skip to content

SMC custom distributions#245

Merged
srba merged 23 commits intoTAPAAL:mainfrom
mtygesen:smc-custom-distributions
Feb 19, 2026
Merged

SMC custom distributions#245
srba merged 23 commits intoTAPAAL:mainfrom
mtygesen:smc-custom-distributions

Conversation

@mtygesen
Copy link
Copy Markdown
Contributor

@mtygesen mtygesen commented Jan 5, 2026

  • Adds support for custom distributions to the GUI

@mtygesen mtygesen changed the title Smc custom distributions SMC custom distributions Jan 5, 2026
@mtygesen mtygesen marked this pull request as ready for review January 27, 2026 15:30
@mtygesen mtygesen marked this pull request as draft January 27, 2026 16:48
@srba srba self-requested a review January 27, 2026 19:06
Copy link
Copy Markdown
Member

@srba srba left a comment

Choose a reason for hiding this comment

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

Custom should not be under discrete distribution. I think that perhaps it should be a third toggle option with custom. When you select it, there should be a dropdown menu with already declared custom distributions from which you can select it. There should also be a button "Manage custom distribution" if you toggle custom distribution. This should open a new menu that will allow to add and delete custom distribution to a drop down menu. When you add a new one, you should be able to give it a name and load it from a file. The file dialog should be the same as if you e.g. open a net - right now the file dialog does not use the native window for locating a file.
The path to the file from where the distribution was loaded should not be displayed/remembered anywhere. There should be also an option to see the values in the distributions and to explore them, possibly edit.

@srba
Copy link
Copy Markdown
Member

srba commented Feb 1, 2026

Once you click on Add new distribution, ask only for the name but then show the Edit window directly where the user can add the values manually. Add the also a button "Load from a file" to this dialog that will open the file dialog and populate the text field with the values from the file.
Also, when you click on "Manage distributions", select (put focus on) the first distribution on the list automatically.
Finally, show the density function looks weird. If I enter e.g. the values 1 2 3 4 5 then it shows that last value with higher probability.

@mtygesen mtygesen marked this pull request as ready for review February 1, 2026 21:15
@srba srba self-requested a review February 7, 2026 12:08
Copy link
Copy Markdown
Member

@srba srba left a comment

Choose a reason for hiding this comment

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

The visualization still does not work, if you enter e.g. the values 1, 2, 3, 4, 5 on the x-axis they will show at 1.4, 2.2, 3.8 and 4.6 but not at the correct values.

Also, to the dialog, please add that we need one number per line
"Enter values (one real number per line) for ... "

@srba srba self-requested a review February 10, 2026 20:15
Copy link
Copy Markdown
Member

@srba srba left a comment

Choose a reason for hiding this comment

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

For urgent transitions it should not be allowed to select custom distribution - open e.g. fireflies and double-click on some urgent transition to see that custom distributions are enabled.

Also, if you make two custom distributions and and choose one and then go to manage distributions and rename the currently selected distribution, then after finishing the custom distribution in the dropdown menu jumps to the other one (but it should show the renamed distribution)

Finally, if you want to add a new distribution to the list, the dialog with the name of the distrubution looks different than normally as it shows the TAPAAL logo. Maybe we could do without it.

Also, the "Edit values (one real number per ...)" does not all show - we need to make the window wider I think

@srba srba self-requested a review February 15, 2026 19:59
Copy link
Copy Markdown
Member

@srba srba left a comment

Choose a reason for hiding this comment

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

The query dialog got too wide (and it increases length if you change from continuous/discrete to custom distribution. We need to have it more narrow so that it fits on the laptop screen.

Also, in the "Mange distribution" if you click on "close" the currently highlighted distribution will get replaced as the current one - even though initially another one was selected in the dropdown menu next to custom distributions. Closing the dialog should now just rewrite the the previously selected value - one should be able to do so only in the dropdown menu.

@srba srba self-requested a review February 16, 2026 18:38
Copy link
Copy Markdown
Member

@srba srba left a comment

Choose a reason for hiding this comment

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

The dialog looks good now. But I found another issue:

You can in the dialog select custom distribution and not define any distribution and click ok. The transition will now say custom(null) distribution and the verification will fail (check e.g. on fireflies). Similarly, if you created a custom distribution but in the "manage distributions" you actually delete it and then close the dialog and press "ok", you have again "custom(null)" distribution which gives verification error.

@srba srba self-requested a review February 18, 2026 10:47
Copy link
Copy Markdown
Member

@srba srba left a comment

Choose a reason for hiding this comment

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

It is still possible to enter costum distribution with (null) argument.
Open e.g. fireflies, open some transition, select "custom" and then click on e.g. <*> or on the button "reset expression" and the "OK" button now gets enabled and you can save it with (null) which gives verification problem.

…ry and custom distribution selected without any valid custom distributions
@srba srba self-requested a review February 19, 2026 09:59
Copy link
Copy Markdown
Member

@srba srba 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 to me.

@srba srba merged commit fed28ed into TAPAAL:main Feb 19, 2026
1 check passed
@mtygesen mtygesen deleted the smc-custom-distributions branch February 19, 2026 12:21
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