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

Fix cpp pointer bug #435

Merged
merged 35 commits into from
Jan 20, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
536e7db
fix doi
martinju Dec 29, 2024
455f425
remove vignette figures from Rbuildignore
martinju Dec 29, 2024
17c4496
starting on cran-comments
martinju Dec 29, 2024
e9b30c0
spelling
martinju Dec 29, 2024
c626e8a
replace sciencedirect link (instead of going doi) with mj.com link
martinju Dec 29, 2024
eb902f4
cran comments
martinju Dec 29, 2024
de9b8c7
Final cran comments. Ready for submission upon merge to master
martinju Dec 29, 2024
54591d2
Merge remote-tracking branch 'origin/master' into CRANsubmission
martinju Jan 7, 2025
8c65bf8
submitted!
martinju Jan 7, 2025
4e8537a
Fix cran note regarding too long cpu time
martinju Jan 7, 2025
0f8b43d
another attempt
martinju Jan 7, 2025
d3bb187
furr to progressr link
martinju Jan 14, 2025
1ae012b
skip parallelization tests on cran
martinju Jan 14, 2025
d52df66
\dontrun on long running examples
martinju Jan 14, 2025
19e9054
.
martinju Jan 14, 2025
cc641d0
forcing single threading for test in testthat.R
martinju Jan 15, 2025
afdbbef
quote shaprpy + Python per CRAN request
martinju Jan 16, 2025
6e85db2
Reduce size by adding snaps and paper folds to rbuildignor
martinju Jan 16, 2025
a8f38fe
repo -> repository
martinju Jan 16, 2025
3c9958d
trying to remove the pointers in the main gaussian cpp function
martinju Jan 17, 2025
5054e82
testing
martinju Jan 17, 2025
2770b43
removing both asym caus tests goes through. now keeping only setup
martinju Jan 18, 2025
1c46922
single commented out test
martinju Jan 18, 2025
c5580af
reverse errorcandidates
martinju Jan 18, 2025
439ea6a
idientified the critical test
martinju Jan 18, 2025
c042aff
bugfix integer(0) -> NULL edge case
martinju Jan 20, 2025
a48f659
temporary global bugfix stuff
martinju Jan 20, 2025
18bd542
unstage tmp globals
martinju Jan 20, 2025
e638541
update test files after bugfix
martinju Jan 20, 2025
8e3e41f
additional equality test for regular condsym
martinju Jan 20, 2025
f5104e7
Merge branch 'master' into fix_cpp_pointer_bug
martinju Jan 20, 2025
91970ac
styler
martinju Jan 20, 2025
abcb25a
update news
martinju Jan 20, 2025
e11dca1
moving back to pointers in the gaussian cpp func
martinju Jan 20, 2025
c4a84c9
rerun vignettes
martinju Jan 20, 2025
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
3 changes: 3 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
# shapr (development version)

* Fix CRAN NOTE which turned out to be bug related to returning NULL rather than integer(0) to identify (unconditional)
asymmetric causal sampling [#435](https://github.com/NorskRegnesentral/shapr/pull/435)

# shapr 1.0.1

* Spelling checking and other minor clean up [#431](https://github.com/NorskRegnesentral/shapr/pull/431))
Expand Down
7 changes: 6 additions & 1 deletion R/asymmetric_and_casual_Shapley.R
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,12 @@ get_S_causal_steps <- function(S, causal_ordering, confounding, as_string = FALS

# If confounding is FALSE, add intervened features in the same component to the `to_condition` set.
# If confounding is TRUE, then no extra conditioning.
if (!confounding[i]) to_condition <- union(intersect(causal_ordering[[i]], index_given), to_condition)

if (!confounding[i]) {
inter0 <- intersect(causal_ordering[[i]], index_given)
inter0 <- if (length(inter0) == 0) NULL else inter0
to_condition <- union(inter0, to_condition)
}

# Save Sbar and S (sorting is for the visual)
to_sample <- sort(to_sample)
Expand Down
1 change: 0 additions & 1 deletion R/explain_forecast.R
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@
#'
#' @author Jon Lachmann, Martin Jullum
#' @examples
#'
#' \dontrun{
#' # Load example data
#' data("airquality")
Expand Down
1 change: 0 additions & 1 deletion R/plot.R
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@
#'
#' @export
#' @examples
#'
#' \dontrun{
#' data("airquality")
#' airquality <- airquality[complete.cases(airquality), ]
Expand Down
1 change: 0 additions & 1 deletion R/zzz.R
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
.onLoad <- function(libname = find.package("shapr"), pkgname = "shapr") {

# CRAN Note avoidance 1
# (as per https://stackoverflow.com/questions/77323811/r-package-to-cran-had-cpu-time-5-times-elapsed-time)
# CRAN OMP THREAD LIMIT
Expand Down
1 change: 0 additions & 1 deletion man/explain_forecast.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion man/plot.shapr.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 5 additions & 5 deletions tests/testthat/_snaps/asymmetric-causal-output.md
Original file line number Diff line number Diff line change
Expand Up @@ -374,11 +374,11 @@

i Using 32 of 32 coalitions.
Output
explain_id none Solar.R Wind Temp Month Day
<int> <num> <num> <num> <num> <num> <num>
1: 1 42.44 -7.978 10.871 12.1981 -2.188 -0.3003
2: 2 42.44 3.637 -6.474 -9.6711 -1.850 0.4779
3: 3 42.44 1.926 -27.039 0.7298 1.404 5.4112
explain_id none Solar.R Wind Temp Month Day
<int> <num> <num> <num> <num> <num> <num>
1: 1 42.44 -8.606 10.387 13.963 -4.010 0.8685
2: 2 42.44 3.506 -6.071 -4.521 -6.183 -0.6104
3: 3 42.44 2.371 -26.300 2.790 1.600 1.9707

# output_sym_caus_conf_mix

Expand Down
Binary file not shown.
2 changes: 1 addition & 1 deletion tests/testthat/test-asymmetric-causal-output.R
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,6 @@ test_that("output_asym_caus_conf_TRUE", {
})



test_that("output_asym_caus_conf_FALSE", {
expect_snapshot_rds(
explain(
Expand Down Expand Up @@ -250,6 +249,7 @@ test_that("output_sym_caus_conf_TRUE", {
)
})


test_that("output_sym_caus_conf_FALSE", {
expect_snapshot_rds(
explain(
Expand Down
30 changes: 30 additions & 0 deletions tests/testthat/test-asymmetric-causal-setup.R
Original file line number Diff line number Diff line change
Expand Up @@ -341,3 +341,33 @@ test_that("asymmetric erroneous input: `confounding`", {
error = TRUE
)
})



test_that("cond_sym_as_NULLconfounding", {
ex_condsym <- explain(
testing = TRUE,
model = model_lm_numeric,
x_explain = x_explain_numeric,
x_train = x_train_numeric,
approach = "gaussian",
phi0 = p0,
n_MC_samples = 5 # Just for speed
)

ex_NULLconfounding <- explain(
testing = TRUE,
model = model_lm_numeric,
x_explain = x_explain_numeric,
x_train = x_train_numeric,
approach = "gaussian",
phi0 = p0,
asymmetric = FALSE,
causal_ordering = list(1:2, 3, 4:5),
confounding = NULL,
n_MC_samples = 5 # Just for speed
)

# When confounding is NULL, causal_ordering is ignored and regular symmetric conditional shapley values is computed
expect_equal(ex_condsym$shapley_values_est, ex_NULLconfounding$shapley_values_est)
})
305 changes: 169 additions & 136 deletions vignettes/asymmetric_causal.Rmd

Large diffs are not rendered by default.

Binary file modified vignettes/figure_asymmetric_causal/compare_plots-1.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified vignettes/figure_asymmetric_causal/explanation_sym_con_SV-1.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified vignettes/figure_asymmetric_causal/group_gaussian_plot_SV-1.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified vignettes/figure_asymmetric_causal/n_coalitions_plot_SV-1.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified vignettes/figure_asymmetric_causal/n_coalitions_plot_beeswarm-1.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified vignettes/figure_asymmetric_causal/scatter_plots-1.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified vignettes/figure_asymmetric_causal/two_dates_1-1.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified vignettes/figure_asymmetric_causal/two_dates_2-1.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified vignettes/figure_asymmetric_causal/two_dates_3-1.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading