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.

Sign up for GitHub

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

Harmonize batch distribution ++ #359

Merged
merged 65 commits into from
Nov 20, 2023
Merged

Harmonize batch distribution ++ #359

Changes from 1 commit
Commits
Show all changes
65 commits
Select commit Hold shift + click to select a range
9578520
Bugfix in `prepare_data()` related to vector of approaches. When usin…
LHBO Aug 24, 2023
8da040d
# Lars have added `n_combinations` - 1 as a possibility, as the funct…
LHBO Aug 24, 2023
e9f6ae9
Samll typo.
LHBO Aug 24, 2023
7412780
Fixed bug. All messages says "n_combinations is larger than or equal …
LHBO Aug 24, 2023
22c8e17
Added script demonstrating the bug that shapr does not enter the exac…
LHBO Aug 31, 2023
a05b82f
Added (tentative) test that checks that shapr enters exact mode when …
LHBO Aug 31, 2023
d0f278d
Added script that demonstrates the bug before the bugfix, and added t…
LHBO Aug 31, 2023
6fd2d91
Fixed lint warnings in `approach.R`.
LHBO Aug 31, 2023
4f0bdb9
Added two parameters to the `internal$parameters` list which contains…
LHBO Sep 4, 2023
2a940bf
Added test to check that `n_batches` must be larger than or equal to …
LHBO Sep 4, 2023
303df5c
Updated `get_default_n_batches` to take into consideration the number…
LHBO Sep 4, 2023
c3c7a87
Changed where seed is set such that it applies for both regular and c…
LHBO Sep 4, 2023
8e6cc9b
Typo
LHBO Sep 4, 2023
246c2cf
Added test to check that setting the seed works for combined approaches.
LHBO Sep 4, 2023
e873f1d
typo in test function
LHBO Sep 4, 2023
5a2c2eb
Added file to demonstrate the bugs (before the bugfix)
LHBO Sep 4, 2023
42c5ed1
Added new test
LHBO Sep 4, 2023
bccf6ff
Updated tests by removing n_samples
LHBO Sep 4, 2023
078c838
Added a bugfix to shapr not using the correct number of batches. Mayb…
LHBO Sep 4, 2023
703b248
Updated the demonstration script
LHBO Sep 4, 2023
c903e6b
Added last test and fixed lintr
LHBO Sep 4, 2023
801ff5f
Lint again.
LHBO Sep 4, 2023
3d216ee
Merge remote-tracking branch 'origin/master' into Lars/bugfix_n_combi…
martinju Sep 5, 2023
9de817f
styler
martinju Sep 5, 2023
d263f37
minor edits to tests
martinju Sep 5, 2023
9fbd49f
Merge branch 'Lars/bugfix_n_combinations' into Lars/bugfix_combined_a…
martinju Sep 5, 2023
14acadc
simplifies comment
martinju Sep 5, 2023
0f617a4
comb files ok
martinju Sep 5, 2023
a0753e5
Merge master into branch
LHBO Sep 5, 2023
b7d4402
Updated bug in independence approach related to categorical features …
LHBO Sep 6, 2023
c28a264
Updated bug in independence approach related to categorical features …
martinju Sep 5, 2023
0bf42bd
Merge branch 'Lars/bugfix_combined_approaches' of https://github.com/…
LHBO Sep 6, 2023
7971eec
lint warning
LHBO Sep 6, 2023
c35e0bc
Lint
LHBO Sep 7, 2023
cb15629
lint
LHBO Sep 7, 2023
6fb5350
Merge with the branch with the bugfix for the combined approaches. No…
LHBO Sep 7, 2023
d1a19b3
updated test files after accepting new values
martinju Sep 9, 2023
a3c238b
adjustments to comments and Lars' TODO-comments
martinju Sep 9, 2023
b0dd3bf
update snapshot file after weight adjustment
martinju Sep 10, 2023
0988ace
cleaned up doc
martinju Sep 10, 2023
f790c22
rerun doc
martinju Sep 10, 2023
f9116db
style
martinju Sep 10, 2023
67e5064
Merge branch 'master' into Lars/bugfix_combined_approaches
martinju Sep 10, 2023
1fa2753
Merge branch 'Lars/bugfix_combined_approaches' into Lars/bugfix_get_d…
martinju Sep 11, 2023
70f353f
Merged master into branch.
LHBO Sep 11, 2023
e0d925d
Changed to `n_batches = 10` in the combined approaches, as the previo…
LHBO Sep 12, 2023
11cf088
Merge branch 'Lars/bugfix_get_default_n_batches' of github.com:LHBO/s…
martinju Nov 16, 2023
0b146bc
accept OK test changes
martinju Nov 16, 2023
fa6a5b9
additonal Ok test files
martinju Nov 16, 2023
c2599fe
change batches in test files
martinju Nov 16, 2023
aacb474
accept new files
martinju Nov 16, 2023
4dd1a86
handle issue with a breaking change update in the testthat package
martinju Nov 17, 2023
30c202d
+ these
martinju Nov 17, 2023
a224648
removing last (unused) input of approach
martinju Nov 20, 2023
b6da078
updating tests
martinju Nov 20, 2023
c9ade53
+ update setup tests/snaps
martinju Nov 20, 2023
4851217
correcting unique length
martinju Nov 20, 2023
cdc624d
update linting and vignette
martinju Nov 20, 2023
8833b0f
update docs
martinju Nov 20, 2023
b865a65
fix example issue
martinju Nov 20, 2023
53c57eb
temporary disable tests on older R systems
martinju Nov 20, 2023
a04f127
remove unecessary if-else test
martinju Nov 20, 2023
79ddd35
data.table style on Lars's batch adjustment suggestion
martinju Nov 20, 2023
4253ef5
del comment
martinju Nov 20, 2023
2fd62b5
lint
martinju Nov 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions R/setup.R
View file
Open in desktop
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We do not need the if-else anymore, so a simpler version of the code could be:

# Get the number of approaches, which is always either one or 
# one less than the number of features if a combination of approaches is used.
internal$parameters$n_approaches <- length(internal$parameters$approach)

# Get the number of unique approaches, as the same
# approach can be used for several feature combination sizes.
internal$parameters$n_unique_approaches <- length(unique(internal$parameters$approach))

Original file line number Diff line number Diff line change
Expand Up @@ -693,15 +693,13 @@ get_default_n_batches <- function(approach, n_combinations) {
suggestion <- ceiling(n_combinations / 10)
this_min <- 10
this_max <- 1000
min_checked <- max(c(this_min, suggestion))
ret <- min(c(this_max, min_checked))
} else {
suggestion <- ceiling(n_combinations / 100)
this_min <- 2
this_max <- 100
min_checked <- max(c(this_min, suggestion))
ret <- min(c(this_max, min_checked))
}
min_checked <- max(c(this_min, suggestion))
ret <- min(c(this_max, min_checked, n_combinations - 1))
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We subtract 1 as thecheck_n_batches() function specifies that n_batches must be strictly less than n_combinations.

message(
paste0(
"Setting parameter 'n_batches' to ", ret, " as a fair trade-off between memory consumption and ",
Expand Down