Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
* Bugfix in `prepare_data()` related to vector of approaches. When using several approaches the old version only used the first approach. Verified this by adding a print in each prepare_data.approach() function and saw that only the first approach in internal$parameters$approach was used. Can maybe remove code comments before pull request is accepted. Maybe a better method to get the approach? Also updated roxygen2 for the function, as it seemed that it was reflecting the old version of shapr(?) due to arguments which are no longer present. However, one then get a warning when creating the roxygen2 documentation. Discuss some solutions as comments below. Discuss with Martin. * Implemented function that computes the MSEv evaluation criterion for all approaches as long as `internal$parameters$output_size == 1`. Need to think about if it is applicable for vector of outputs. * Plot the MSEv evaluation criterion. This function is a separate plot function and is not part of the `shapr.plot()` function. It would maybe nice to make it a part of it and using, e.g., `plot_type = "MSEv". However, `make_MSEv_evaluation_criterion_plots()` handles list of explanation objects while `shapr.plot()` is restricted to a single shapr explanation object. Thus, one would need to rewrite the `shapr.plot()` to also handle mulitple objects. * Added a section about the MSEv criterion in the vignette. This is s adraft and might need some polishing. * # Lars have added `n_combinations` - 1 as a possibility, as the function `check_n_batches` threw an error for the vignette with gaussian approach with `n_combinations` = 8 and `n_batches = NULL`, as this function here then set `n_batches` = 10, which was too large. We subtract 1 as `check_n_batches` function specifies that `n_batches` must be strictly less than `n_combinations`. * Samll typo. * Fixed bug. All messages says "n_combinations is larger than or equal to 2^m", but all the test only tested for "larger than". I.e., if the user specified n_combinations = 2^m in the call to shapr::explain, the function would not treat it as exact. * Changed to exclude the coalitions which are not effected by the approach by default. Futhermore, there was a logical error. * Added example to the roxygen. This is technically not needed as it is an internal function. Could make a test out of the examples too. * Added some parameters to the plot function to make it more flexiable and to make it easier for the user to controll what figures that are made * Updated code as I had only changed the legend for the fill (which works with bars), but not the col (which works for lines/points). * Added the updated namespace file. Maybe that is why the tests on github did not work beforehand. * Added script demonstrating the bug that shapr does not enter the exact mode when `n_combinations = 2^m`, before the bugfix. * Added (tentative) test that checks that shapr enters exact mode when `n_combinations >= 2^m`. Remove the large comment after discussing that with Martin. * Added script that demonstrates the bug before the bugfix, and added test checking that we do not get an error when runing the code after the bugfix has been applied. * Fixed lint warnings in `approach.R`. * Added `ctree` in the example in roxygen * Add manuals for the two funtions realted to MSEv criterion. * Updated plot function (fixed some inconsistencies) and added detailed examples. Plan to make tests out of them. * Started to make tests, but ran into a shapr bug. * Added two parameters to the `internal$parameters` list which contains the number of approaches and the number of unique approaches. This is for example useful to check that the provided `n_batches` is a valid value. (see next commits) * Added test to check that `n_batches` must be larger than or equal to the number of unique approaches. Before the user could, e.g., set `n_batches = 2`, but use 4 approaches and then shapr would use 4 but not update `n_batches` and without giwing a warning to the user. * Updated `get_default_n_batches` to take into consideration the number of unique approaches that is used. This was not done before and gave inconsistency in what number shapr would reccomend and use when `n_batches` was set to `null` by the user. * Changed where seed is set such that it applies for both regular and combined approaches. Furthermore, added if test, because previous version resulted in not reproducible code, as setting seed to `null` ruins that we set seed in `explain()`. Just consider this small example: # Set seed to get same values twice set.seed(123) rnorm(1) # Seting the same seed gives the same value set.seed(123) rnorm(1) # If we also include null then the seed is removed and we do not get the same value set.seed(123) set.seed(NULL) rnorm(1) # Setining seed to null actually gives a new "random" number each time. set.seed(123) set.seed(NULL) rnorm(1) * Typo * Added test to check that setting the seed works for combined approaches. * typo in test function * Added file to demonstrate the bugs (before the bugfix) * Added new test * Updated tests by removing n_samples * Added a bugfix to shapr not using the correct number of batches. Maybe not the most elegant solution. * Updated the demonstration script * Added last test and fixed lintr * Lint again. * styler * minor edits to tests * simplifies comment * comb files ok * Updated bug in independence approach related to categorical features which caused shapr to crash later. Added comments when I debuged to understand what was going on. I have added some comments about some stuff I did no understand/agree with. Discuss with Martin and correct this before merge. * Updated bug in independence approach related to categorical features which caused shapr to crash later. Added comments when I debuged to understand what was going on. I have added some comments about some stuff I did no understand/agree with. Discuss with Martin and correct this before merge. * lint warning * Lint * lint * Updated roxygen * Added plot test functions for MSEv criterion * Ran styler * Updated most lintr. I disagree that variable names should be less than 30 characters. Discuss with Martin. And it was Stylr that made the changes that caused the brace_linter warnings. * Previous version would not test the output but rather that check that shapr would stop as `n_batches` was less than the length of `approch`. * Added some extra parameters to the one test function. * Updated some parameters in the MSEv test plots. Looked at all of them and they look to do what I want. * SupressMessages to run `testthat::test_file` without messages, and changed tha `bar_plot_MSEv` such that we do not get printed `NULL` to the console when running the test. * Add test images (get the same each time I run `testthat::test_file()`). * Fixed lintr and styler * Updated vignette with MSEv. Fixed lintr and ensured that the figures look like what they are supposed to. * updated test files after accepting new values * adjustments to comments and Lars' TODO-comments * update snapshot file after weight adjustment * cleaned up doc * rerun doc * style * Changed to `n_batches = 10` in the combined approaches, as the previous value (`n_batches = 1`) is not allowed anymore as it is lower than the number of unique used approaches. * Updated some messages in plot * Minor updates to `make_MSEv_evaluation_criterion_plots` * Update the manuals * Updated the MSEv text in the vignette. * accept OK test changes * additonal Ok test files * change batches in test files * accept new files * handle issue with a breaking change update in the testthat package * + these * removing last (unused) input of approach * updating tests * + update setup tests/snaps * correcting unique length * update linting and vignette * update docs * fix example issue * temporary disable tests on older R systems * remove unecessary if-else test * data.table style on Lars's batch adjustment suggestion * del comment * lint * snaps + test adjustment * update plotting snaps to make tests pass * update vignette * . * Renamed `compute_MSEv_evaluation_criterion()` to `compute_MSEv_eval_crit()`. * Renamed `MSEv_evaluation_criterion_for_each_coalition` to `MSEv_eval_crit_each_comb`. * Shorted function/variable names to less than 30 char. * Renamed `MSEv_eval_crit_comb` to `MSEv_eval_crit_combination` for consistency. * Added option for using Shapley weights in `compute_MSEv_eval_crit()` * update snaps after name change * update function names in tests * new figure snaps * update function names in vignette * man + example bug * Fixed places where variable names where not updated * Manual updates * Added `library(shapr)` in examples. * Updated combined approach in `make_MSEv_eval_crit_plots` example to support new version. * Export `compute_MSEv_eval_crit` to make `devtools::run_examples()` run. * Added support for MSEv criterion in `explain()`. * Updated MSEv criterion based on feedback. Now internal. * Added checks for MSEv criterion * Refactored the plot function for MSEv based on feedback * Updated vignette to reflect changes in MSEv criterion code. * Fixed MSEv_combination. Now taking mean of it gives MSEv and we see the weighted importance of the combination on the overall precission when plotting. * Removed default palette and refactored the code. * Updated the vignette to reflect the changes in MSEv criterion. * Set default MSEv values in `setup()` as `explain_forecast()` do not specify them. * Added tests for `MSEv_skip_empty_full_comb` and `MSEv_uniform_comb_weights`. Matched the setup for other logical arguments. * Added test for when MSEv criterion use the Shapley kernel weights * Fixed bug in MSEv_plot function when specifying geom_col_width * Fixed tests for MSEv plot function * Added MSEv references * Added Lars as Author * Removed `MSEv_skip_empty_full_comb` form `explain` and all other functions/tests. * Fixed inncorrect standard deviations of the mean. Now divides by sqrt(n_explain). And better documentation. * MSEv_plots: introduced approximate CI, reduced example, improved documentation * Updated MSEv_plot documentation * Added example file for how to change the MSEv plots * Inserted `level = 0.95` the places it where missing. * Added CI explanation in vignette. * Fix linting and stylr. * Updated manuals (done autmatically by `devtools::run_examples()`) * Added snaps plot svg files * æ ø * update test files * del unused snaps * typo * man * styler * bugfix name in testfile * fixing check warning of missing ... description * fix check note on NSE * Renamed `make_MSEv_eval_crit_plots` into `plot_MSEv_eval_crit` * Changed default confidence level to check for the number of explicands * Renamed the parameter `level` into `CI_level` in the `plot_MSEv_eval_crit` function. * Updated plot_MSEv_eval_crit.Rd and namespace * Introduced `plot_type` parameter in `plot_MSEv_eval_crit()` and updated title * styler * Updated manual * Updated from ifelse to if else to support NULL return * swap * + space with ~ in bquote to fix spacing in svg files * update svg files * man * bugfix vignette + remove unecessary plot arguments * fix NSE check issues --------- Co-authored-by: Martin <[email protected]>
- Loading branch information