Skip to content

Commit

Permalink
Remove unnecessary comments with #browser or #TF1/2
Browse files Browse the repository at this point in the history
  • Loading branch information
njtierney committed Nov 6, 2024
1 parent 80192ed commit 5055a94
Show file tree
Hide file tree
Showing 10 changed files with 9 additions and 77 deletions.
9 changes: 0 additions & 9 deletions R/calculate.R
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,6 @@ calculate_greta_mcmc_list <- function(target,
}

calculate_list <- function(target, values, nsim, tf_float, env) {
### browser()
fixed_greta_arrays <- list()

values_exist <- !identical(values, list())
Expand All @@ -409,7 +408,6 @@ calculate_list <- function(target, values, nsim, tf_float, env) {
dag <- dag_class$new(all_greta_arrays, tf_float = tf_float)

stochastic <- !is.null(nsim)
### browser()
if (stochastic) {

check_if_unsampleable_and_unfixed(fixed_greta_arrays, dag)
Expand Down Expand Up @@ -459,7 +457,6 @@ calculate_target_tensor_list <- function(
nsim
) {
# define the dag and TF graph
### browser()
# change dag mode to sampling
dag$mode <- "all_sampling"

Expand Down Expand Up @@ -498,18 +495,12 @@ calculate_target_tensor_list <- function(
# approaches (in as_tf_function + generate_log_prob_function)
dag$define_tf(target_nodes = target_nodes)

# browser()
# look up the tf names of the target greta arrays (under sampling)
# create an object in the environment that's a list of these, and sample that
target_nodes <- lapply(target, get_node)
target_names_list <- lapply(target_nodes, dag$tf_name)

## TF1/2 OK so the error with Wishart and cholesky is happening here
## I feel as thought this isn't the "problem" per se, but it is where the
## matrix of 1s is returned. So seems to me we first expose the problem here
target_tensor_list <- lapply(target_names_list, get, envir = tfe)
## It looks like it is getting all_sampling_operation_2 and not
## all_sampling_operation_1
target_tensor_list_array <- lapply(target_tensor_list, as.array)

return(target_tensor_list_array)
Expand Down
4 changes: 0 additions & 4 deletions R/dag_class.R
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ dag_class <- R6Class(
initialize = function(target_greta_arrays,
tf_float = "float32",
compile = FALSE) {
# browser()
# build the dag
self$build_dag(target_greta_arrays)

Expand Down Expand Up @@ -171,7 +170,6 @@ dag_class <- R6Class(
# if it's an operation, see if it has a distribution (for lkj and
# wishart) and get mode based on whether the parent has a free state
if (node_type == "operation") {
# browser()
parent_name <- node$parents[[1]]$unique_name
parent_stateless <- parent_name %in% stateless_names
to_sample <- has_distribution(node) & parent_stateless
Expand Down Expand Up @@ -345,9 +343,7 @@ dag_class <- R6Class(
}

# define all nodes in the environment and on the graph
## HERE
lapply(target_nodes, function(x){
# browser()
x$define_tf(self)
})

Expand Down
7 changes: 0 additions & 7 deletions R/node_class.R
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ node <- R6Class(
dim = NA,
distribution = NULL,
initialize = function(dim = NULL, value = NULL) {
## browser()
dim <- dim %||% c(1,1)

# coerce dim to integer
Expand All @@ -36,8 +35,6 @@ node <- R6Class(

# recursively register self and family
register_family = function(dag) {
## TF1/2
## Rename with an explaining variable
## TODO add explaining variable
if (!(self$unique_name %in% names(dag$node_list))) {

Expand Down Expand Up @@ -75,9 +72,6 @@ node <- R6Class(
node$remove_child(self)
},
list_parents = function(dag) {
## TF1/2
## tf_cholesky
## is there a way here to add some check for cholesky?
parents <- self$parents

# if this node is being sampled and has a distribution, consider
Expand Down Expand Up @@ -200,7 +194,6 @@ node <- R6Class(
lapply(
parents[which(!parents_defined)],
function(x){
# browser()
x$define_tf(dag)
}
)
Expand Down
45 changes: 4 additions & 41 deletions R/node_types.R
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ data_node <- R6Class(
inherit = node,
public = list(
initialize = function(data) {
## browser()
# coerce to an array with 2+ dimensions
data <- as_2d_array(data)

Expand Down Expand Up @@ -150,26 +149,7 @@ operation_node <- R6Class(
mode <- dag$how_to_define(self)
# if sampling get the distribution constructor and sample this
if (mode == "sampling") {
# browser()
tensor <- dag$draw_sample(self$distribution)

# if (has_representation(self, "cholesky")) {
# browser()
# cholesky_tensor <- tf_chol(tensor)
# # cholesky_tf_name <- dag$tf_name(self$representation$cholesky)
# cholesky_node <- get_node(representation(self, "cholesky"))
# cholesky_tf_name <- dag$tf_name(cholesky_node)
# assign(cholesky_tf_name, cholesky_tensor, envir = tfe)
## TF1/2
## This assignment I think is supposed to be passed down to later on
## in the script, as `cholesky_tf_name` gets overwritten
# cholesky_tf_name <- dag$tf_name(self)
# tf_name <- cholesky_tf_name
# tensor <- cholesky_tensor
# cholesky_tensor <- tf_chol(tensor)
# cholesky_tf_name <- dag$tf_name(self$representation$cholesky)
# assign(cholesky_tf_name, cholesky_tensor, envir = dag$tf_environment)
# }
}

if (mode == "forward") {
Expand All @@ -188,11 +168,9 @@ operation_node <- R6Class(
operation <- eval(parse(text = self$operation),
envir = self$tf_function_env
)
# browser()
tensor <- do.call(operation, tf_args)
}

# browser()
# assign it in the environment
assign(tf_name, tensor, envir = dag$tf_environment)
}
Expand All @@ -212,7 +190,6 @@ variable_node <- R6Class(
upper = Inf,
dim = NULL,
free_dim = prod(dim)) {
## browser()
check_if_lower_upper_numeric(lower, upper)

# replace values of lower and upper with finite values for dimension
Expand Down Expand Up @@ -283,33 +260,24 @@ variable_node <- R6Class(
mode <- dag$how_to_define(self)

if (mode == "sampling") {
# browser()
distrib_node <- self$distribution

if (is.null(distrib_node)) {
# does it have an anti-representation where it is the cholesky?
# the antirepresentation of cholesky is chol2symm
# if it does, we will take the antirepresentation and get it to `tf` itself
# then we need to get the tf_name
# if yes, we take antirep and get it to `tf`, then get the tf_name
chol2symm_ga <- self$anti_representations$chol2symm
chol2symm_existing <- !is.null(chol2symm_ga)

if (chol2symm_existing) {

chol2symm_node <- get_node(chol2symm_ga)
chol2symm_name <- dag$tf_name(chol2symm_node)
chol2symm_tensor <- get(chol2symm_name, envir = dag$tf_environment)
tensor <- tf_chol(chol2symm_tensor)

}

# chol2symm_ga$define_tf(dag)
# } else {
#
# # if the variable has no distribution create a placeholder instead
# # (the value must be passed in via values when using simulate)
# shape <- to_shape(c(1, self$dim))
# # TF1/2 check
# # need to change the placeholder approach here.
# # NT: can we change this to be a tensor of the right shape with 1s?
# tensor <- tensorflow::as_tensor(1L, shape = shape, dtype = tf_float())
} else {
tensor <- dag$draw_sample(self$distribution)
}
Expand Down Expand Up @@ -432,10 +400,8 @@ distribution_node <- R6Class(
discrete = FALSE,
multivariate = FALSE,
truncatable = TRUE) {
## browser()
super$initialize(dim)

## browser()
# for all distributions, set name, store dims, and set whether discrete
self$distribution_name <- name
self$discrete <- discrete
Expand All @@ -462,7 +428,6 @@ distribution_node <- R6Class(

# create a target variable node (unconstrained by default)
create_target = function(truncation) {
##browser()
vble(truncation, dim = self$dim)
},
list_parents = function(dag) {
Expand Down Expand Up @@ -494,7 +459,6 @@ distribution_node <- R6Class(

# create target node, add as a parent, and give it this distribution
add_target = function(new_target) {
##browser()
# add as target and as a parent
self$target <- new_target
self$add_parent(new_target)
Expand Down Expand Up @@ -621,7 +585,6 @@ node_classes_module <- module(

# shorthand for distribution parameter constructors
distrib <- function(distribution, ...) {
##browser()
check_tf_version("error")

# get and initialize the distribution, with a default value node
Expand Down
6 changes: 0 additions & 6 deletions R/probability_distributions.R
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ normal_distribution <- R6Class(
inherit = distribution_node,
public = list(
initialize = function(mean, sd, dim, truncation) {
## browser()
mean <- as.greta_array(mean)
sd <- as.greta_array(sd)

Expand Down Expand Up @@ -1051,10 +1050,6 @@ wishart_distribution <- R6Class(
input_output_cholesky = TRUE
)

## TF1/2
## The issue with getting the cholesky part of the Wishart
## isn't happening here,
## This produces something that looks about right
chol_draws <- chol_distrib$sample(seed = seed)

# equivalent to (but faster than) tf_chol2symm(tf_transpose(chol_draws))
Expand Down Expand Up @@ -1404,7 +1399,6 @@ uniform <- function(min, max, dim = NULL) {
#' @rdname distributions
#' @export
normal <- function(mean, sd, dim = NULL, truncation = c(-Inf, Inf)) {
##browser()
distrib("normal", mean, sd, dim, truncation)
}

Expand Down
2 changes: 0 additions & 2 deletions R/unknowns_class.R
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,6 @@ print.unknowns <- function(x, ..., n = 10) {
# set NA values to ? for printing
x[is.na(x)] <- " ?"

# browser()

n_print <- getOption("greta.print_max") %||% n

n_unknowns <- length(x)
Expand Down
6 changes: 3 additions & 3 deletions R/write-logfiles.R
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ write_greta_install_log <- function(path = greta_logfile) {
<h1>Greta installation logfile</h1>
<h2>Created: {{sys_date}}</h2>
<p>Use this logfile to explore potential issues in installation with greta</p>
<p>Try opening this in a browser and searching the text for "error" with Cmd/Ctrl+F</p>
<p>Try opening this in a HTML browser and searching the text for "error" with Cmd/Ctrl+F</p>

Check warning on line 40 in R/write-logfiles.R

View check run for this annotation

Codecov / codecov/patch

R/write-logfiles.R#L40

Added line #L40 was not covered by tests
<h2>Miniconda</h2>
Expand Down Expand Up @@ -142,15 +142,15 @@ sys_get_env <- function(envvar){
#' Read a greta logfile
#'
#' This is a convenience function to facilitate reading logfiles. It opens
#' a browser using [utils::browseURL()]. It will search for
#' a HTML browser using [utils::browseURL()]. It will search for
#' the environment variable "GRETA_INSTALLATION_LOG" or default to
#' `tools::R_user_dir("greta")`. To set
#' "GRETA_INSTALLATION_LOG" you can use
#' `Sys.setenv('GRETA_INSTALLATION_LOG'='path/to/logfile.html')`. Or use
#' [greta_set_install_logfile()] to set the path, e.g.,
#' `greta_set_install_logfile('path/to/logfile.html')`.
#'
#' @return opens a URL in your default browser
#' @return opens a URL in your default HTML browser.
#' @export
open_greta_install_log <- function(){

Expand Down
4 changes: 2 additions & 2 deletions man/open_greta_install_log.Rd

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

2 changes: 0 additions & 2 deletions tests/testthat/test_posteriors_geweke.R
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
Sys.setenv("RELEASE_CANDIDATE" = "false")

## TF1/2 - method for this test needs to be updated for TF2
## See https://github.com/greta-dev/greta/issues/720
test_that("samplers pass geweke tests", {
skip_if_not(check_tf_version())

Expand Down
1 change: 0 additions & 1 deletion tests/testthat/test_posteriors_standard_uniform.R
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
Sys.setenv("RELEASE_CANDIDATE" = "true")

## TF1/2 this sampler fails
test_that("samplers are unbiased for standard uniform", {
skip_if_not(check_tf_version())

Expand Down

0 comments on commit 5055a94

Please sign in to comment.