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

Refactoring exposure function #42

Merged
merged 17 commits into from
Nov 12, 2024
Merged

Conversation

gvegayon
Copy link
Member

@gvegayon gvegayon commented Oct 29, 2024

This pull request includes several changes to the diffnet package, focusing on enhancing the exposure calculation functionality, adding support for multiple diffusion processes, and improving the testing framework. The most significant changes include adding a new author, updating the version metadata, modifying the exposure calculation logic, and adding comprehensive tests.

Metadata and Documentation Updates:

  • Added a new author, Anibal Olivera Morales, to the DESCRIPTION file.
  • Updated the version metadata in the R/diffnet-class.r file to version 5.
  • Corrected and clarified the documentation for the dgr.array function in R/stats.R.

Exposure Calculation Enhancements:

  • Refactored the exposure calculation logic in R/stats.R to handle multiple diffusion processes and added normalization steps. [1] [2]
  • Improved the exposure_for function to support multiple contagions and added checks for attribute dimensions.

Testing Improvements:

  • Added new test cases in tests/testthat/test-stats.R to validate the multidiffusion exposure calculations, including handling of lagged exposures and structural equivalence.
  • Introduced additional test files in the playground directory to discuss and validate exposure calculations (exposure-ans-discussion.R, exposure-out-discussion.R, multidiff-test-discussion.R). [1] [2] [3]

@gvegayon gvegayon linked an issue Oct 29, 2024 that may be closed by this pull request
@aoliveram aoliveram changed the title Adding myself to the project Anibal O. - discussions and modifications Oct 31, 2024
@gvegayon gvegayon force-pushed the 41-refactor-cumadopt-and-adopt branch from a54800b to 587babb Compare November 6, 2024 18:36
@gvegayon gvegayon changed the title Anibal O. - discussions and modifications Refactoring exposure function Nov 6, 2024
@gvegayon gvegayon marked this pull request as ready for review November 6, 2024 18:37
Copy link
Member Author

@gvegayon gvegayon left a comment

Choose a reason for hiding this comment

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

Address the missing bits and we will merge.

DESCRIPTION Outdated
@@ -7,6 7,7 @@ Authors@R: c(
),
person("Thomas", "Valente", email="[email protected]", role=c("aut", "cph"),
comment=c(ORCID="0000-0002-8824-5816", what="R original code")),
person("Anibal", "Olivera Morales", role = c("aut", "ctb")),
Copy link
Member Author

Choose a reason for hiding this comment

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

Add your ORCID like I do.

@@ -631,6 631,7 @@ new_diffnet <- function(graph, toa, t0=min(toa, na.rm = TRUE), t1=max(toa, na.rm
as.character(name)))
meta$behavior <- ifelse(!length(behavior), "", ifelse(is.na(behavior), "",
as.character(behavior)))
meta$version <- 5
Copy link
Member Author

Choose a reason for hiding this comment

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

There's a way of capturing the package version.

Suggested change
meta$version <- 5
meta$version <- utils::packageVersion("netdiffuseR")

R/stats.R Outdated
}

#as.vector(ans)
return(as.vector(ans))
}

# library(microbenchmark)
Copy link
Member Author

Choose a reason for hiding this comment

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

Suggested change
# library(microbenchmark)

R/stats.R Outdated
Comment on lines 676 to 686
#out <- matrix(nrow = nrow(cumadopt), ncol = ncol(cumadopt))

#if (lags >= 0L) {
# for (i in 1:(nslices(graph) - lags))
# out[,i lags]<- .exposure(graph[[i]], cumadopt[,i,drop=FALSE], attrs[,i,drop=FALSE],
# outgoing, valued, normalized, self)
#} else {
# for (i in (1-lags):nslices(graph))
# out[,i lags]<- .exposure(graph[[i]], cumadopt[,i,drop=FALSE], attrs[,i,drop=FALSE],
# outgoing, valued, normalized, self)
#}
Copy link
Member Author

Choose a reason for hiding this comment

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

Suggested change
#out <- matrix(nrow = nrow(cumadopt), ncol = ncol(cumadopt))
#if (lags >= 0L) {
# for (i in 1:(nslices(graph) - lags))
# out[,i lags]<- .exposure(graph[[i]], cumadopt[,i,drop=FALSE], attrs[,i,drop=FALSE],
# outgoing, valued, normalized, self)
#} else {
# for (i in (1-lags):nslices(graph))
# out[,i lags]<- .exposure(graph[[i]], cumadopt[,i,drop=FALSE], attrs[,i,drop=FALSE],
# outgoing, valued, normalized, self)
#}

R/stats.R Outdated
Comment on lines 479 to 486
#ans <- ( graph %*% (attrs * cumadopt) )
#
#if (normalized) {
# as.vector(ans/( graph %*% attrs 1e-20 ))
#} else {
# as.vector(ans)
#}
#
Copy link
Member Author

Choose a reason for hiding this comment

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

Suggested change
#ans <- ( graph %*% (attrs * cumadopt) )
#
#if (normalized) {
# as.vector(ans/( graph %*% attrs 1e-20 ))
#} else {
# as.vector(ans)
#}
#

Copy link
Member Author

Choose a reason for hiding this comment

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

Delete this file.

@gvegayon gvegayon merged commit 7bd4c98 into master Nov 12, 2024
4 checks passed
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.

Refactor cumadopt and adopt
2 participants