Skip to content
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

predictive sliders improve parser #173

Merged
merged 4 commits into from
Jan 10, 2023
Merged

predictive sliders improve parser #173

merged 4 commits into from
Jan 10, 2023

Conversation

aloctavodia
Copy link
Contributor

This provides a simpler parser. This is possible because now we are using the relevant variables from the signature and instantiating distributions at their default values. But this is not just a refactor, this also provides more flexible (and "natural") way of specifying the model. For example, this is a valid syntax b = pz.Normal(np.exp(a)*x, c).rvs(), when previously the np.exp would have needed to be applied on a separate line.

This also tries to be more "clever" and determine the boundary information (the info in parentheses next to the sliders name) of the parameters not only from the information of the parameter itself (e.g. a scale parameter, restricted to be positive), but also of the functions applied to them.

For instance a = pz.Normal(0, c).rvs(), c is restricted to be positive here, but it can be negative here a = pz.Normal(0, np.exp(c)).rvs() . Providing a truly general solution will probably be too hard, and it may require a different approach to parse the function. Something easier may be to solve a few common cases and for those that can not be solved return "(unk, unk)", signaling that the function is not able to guess the boundaries of the parameter. This will be complemented by allowing the users to modify the min and max values of each slider directly using text boxes, and/or passing a dictionary.

@codecov-commenter
Copy link

codecov-commenter commented Jan 10, 2023

Codecov Report

Merging #173 (c60cddb) into main (915be0c) will decrease coverage by 0.20%.
The diff coverage is 79.48%.

@@            Coverage Diff             @@
##             main     #173      +/-   ##
==========================================
- Coverage   86.29%   86.09%   -0.21%     
==========================================
  Files          26       26              
  Lines        2474     2488      +14     
==========================================
+ Hits         2135     2142       +7     
- Misses        339      346       +7     
Impacted Files Coverage Δ
preliz/internal/plot_helper.py 93.20% <50.00%> (-2.70%) ⬇️
preliz/internal/parser.py 94.44% <92.30%> (-2.53%) ⬇️
preliz/predictive/predictive_sliders.py 92.30% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@aloctavodia aloctavodia merged commit e969701 into main Jan 10, 2023
@aloctavodia aloctavodia deleted the pred_sl branch January 10, 2023 14:14
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