Solves differences between C++ and R estimation engines #84

auzaheta · 2023-06-20T20:27:11Z

Description

Solves startTime and endTime bug on DyNAM and REM models
preprocessing.
Clean unnecessary functions imports.
Solves aes_string() deprecation.
Solves issue on C++ engine on DyNAM-rate.
Enforcing an 80-character line length on C++ code.
Comply with code style.
Changes that reduce execution time in the R estimation engine.

Fixes #71
Closes #39

Checklist:

I have added tests that prove my fix is effective or that my feature works
The package builds on my OS without issues (please add workstation details)
My changes generate no new warnings
My code follows the style guidelines of this project
I have commented on my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation (.R, NEWS.md)
I have bumped the version by the appropriate increment in the DESCRIPTION file (major, minor, patch)

…ead of ggplot2::aes_string

Merge branch 'develop' of github.com:snlab-ch/goldfish into develop # Conflicts: # R/functions_diagnostics.R

- Style C++ code to 80 columns - Debug wrong update in compositional change second mode

- Solves bug when reduce updates with duplicated entries

jhollway · 2023-06-21T12:58:33Z

vignettes/teaching2.Rmd.orig

+For more, please see Hollway (2020) Network Embeddedness and
+the Rate of Water Cooperation and Conflict.


Hollway, James. 2020. “Network Embeddedness and the Rate of Water Cooperation and Conflict.” Pp. 87–113 in Networks in Water Governance, edited by M. Fischer and K. Ingold. Cham: Springer International Publishing.

jhollway · 2023-06-21T13:00:46Z

vignettes/dynami-example.R

 library(goldfish)
 data("RFID_Validity_Study")
 #?RFID_Validity_Study


-## -----------------------------------------------------------------------------------------------------------
+## ----------------------------------------------------------------------------------------------------------------------
 head(participants)



I wonder whether we should change all these vignettes to tutorials. I think tutorials better fits how we use tutorials in courses, for example, and helps trim the size/testing burden of the package too. See manynet/migraph for how we have done it there.

jhollway · 2023-06-21T13:01:57Z

vignettes/goldfishEffects.Rmd

 This vignette summarizes effects that are implemented in `goldfish` and
-are thus available to be used with actor-oriented DyNAM models and tie-oriented Relational Event Models.
+are thus available to be used with actor-oriented DyNAM models and
+tie-oriented Relational Event Models.


Is there anything actually run in this vignette, or is it 'just' an information page.

jhollway · 2023-06-21T13:04:22Z

vignettes/teaching1.R

  add_node_attribute("floor", actors$floor)

 autographr(callNetworkHlf, labels = FALSE, layout = "fr") +
  geom_node_point(aes(color = as.factor(floor)), size = 2, show.legend = FALSE)

 # The network at the end
-callNetworkEnd <- as.matrix(callNetwork, time = max(calls$time) + 1) |> 
+callNetworkEnd <- as.matrix(callNetwork, time = max(calls$time) + 1) |>
  add_node_attribute("floor", actors$floor)

 autographr(callNetworkEnd, labels = FALSE, layout = "fr") +


Using vignettes means the plots will be changed everytime they are run, inflating the repository etc for no good reason. If they are illustrative, can they be moved to the README? If they are for learning/discovery, can they be in tutorials?

jhollway · 2023-06-21T13:06:48Z

tests/testthat/test-effects_preprocessing_DyNAM_rate.R

+test_that(
+  "out/in/deg weighted right censored preprocessing",


Excellent job adding all these very important tests, @auzaheta , thank you!

jhollway · 2023-06-21T13:11:42Z

R/functions_diagnostics.R

-#' @importFrom changepoint cpt.mean cpt.var
-#' @importFrom ggplot2 ggplot aes geom_line geom_point theme_minimal xlab ylab
-#'  geom_vline scale_x_continuous theme element_text


These still seem to be required below, e.g. lines 208 and 212

jhollway · 2023-06-21T13:12:58Z

R/functions_effects_DyNAM_choice.R

+  if ("cache" %in% .argsNames) cache <- stats
+  else cache <- NULL


If we are keen to use {rlang} as a dependency, there is also rlang::%||%, which might be useful here.

jhollway · 2023-06-21T13:14:04Z

R/functions_group_interaction_utility.R

@@ -52,7 +51,7 @@ defineGroups_interaction <- function(records, actors, seed.randomization,
  stopifnot(
    inherits(records, "data.frame"),
    inherits(actors, "data.frame"),
-    is(seed.randomization, "numeric"),
+    methods::is(seed.randomization, "numeric"),


I think a methods dependency needs to be declared?

auzaheta and others added 17 commits September 6, 2022 22:21

Solves #39 tested on DyNAM-choice

1ad9949

Solves #39 tested on DyNAM-rate

50625b7

Solves aes_string() deprecation

b0e1aab

Clean unnecessary import from in namespace

4a4cc11

examineOutliers and examineChangepoints now rely on ggplot2::aes inst…

2b45b0f

…ead of ggplot2::aes_string

Add rlang to avoid no visible binding for global variable

a8f7dfb

Merge branch 'develop' of github.com:snlab-ch/goldfish into develop # Conflicts: # R/functions_diagnostics.R

Solve merging

31d49c4

Reduce stats update size keeping only last update for a dyad

42f62b9

Solves #71 due to use of wrong index and non-passing argument.

93ffc55

- Style C++ code to 80 columns - Debug wrong update in compositional change second mode

Debug wrong compositional change update for two-mode

c5d1519

Enforcing 80-character line lenght on C++ code

84566d8

- Solves bug when reduce updates with duplicated entries

Delete deprecated src files

b0d4012

Complaining with code style

4ec201e

Clean R engine checking use of compositional change

6a5fd17

Debug gather DyNAM-rate and add testing C++ engine

646d365

Update vignettes

cdd08bc

Correct typos on NEWS

52f6eab

auzaheta requested a review from jhollway June 20, 2023 20:27

auzaheta self-assigned this Jun 20, 2023

Update GitHub actions

83052ff

jhollway approved these changes Jun 21, 2023

View reviewed changes

jhollway reviewed Jun 21, 2023

View reviewed changes

auzaheta merged commit 74481fa into main Jun 21, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Solves differences between C++ and R estimation engines #84

Solves differences between C++ and R estimation engines #84

auzaheta commented Jun 20, 2023

jhollway Jun 21, 2023

jhollway Jun 21, 2023

jhollway Jun 21, 2023

jhollway Jun 21, 2023

jhollway Jun 21, 2023

jhollway Jun 21, 2023

jhollway Jun 21, 2023

jhollway Jun 21, 2023

		For more, please see Hollway (2020) Network Embeddedness and
		the Rate of Water Cooperation and Conflict.

		test_that(
		"out/in/deg weighted right censored preprocessing",

		if ("cache" %in% .argsNames) cache <- stats
		else cache <- NULL

Solves differences between C++ and R estimation engines #84

Solves differences between C++ and R estimation engines #84

Conversation

auzaheta commented Jun 20, 2023

Description

Checklist:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment