Review #4
Review #4
Conversation
| screen_index <- function(simulator) { | ||
| method = simulator$setting[["method"]] | ||
| if (method != "Random" && method != "Expert") { | ||
| metric_fun = switch(simulator$setting[["method"]], |
There was a problem hiding this comment.
It seems better to define a get_metric_fun function instead.
There was a problem hiding this comment.
This module is only used once. It would be necessary to put it in a function if it were used in multiple places.
There was a problem hiding this comment.
Correct, that's one of the reasons for adding a function, the other one being readability.
| iter_num = simulator$setting[["iter_num"]] | ||
| simulator$errors = as.numeric() | ||
| prepare_data(simulator) | ||
| for (i in 1:iter_num) { |
There was a problem hiding this comment.
very complex, nested for loop. Simplify this (maybe use Extract Method with good function names to help understand this.
There was a problem hiding this comment.
The nested for loop has two levels. One is for (i in 1:iter_num) and the other is for (gene_id in gene_ids). The meaning of each level is clear, and not so complicated, although nested for loop is not encouraged.
There was a problem hiding this comment.
I still think this should change.
|
|
||
| compute_MI_denominator <- function(simulator){ | ||
|
|
||
| d = simulator$model[["numDim"]] |
There was a problem hiding this comment.
renamed or at least put a comment on what it means.
There was a problem hiding this comment.
renamed it as num_of_features.
There was a problem hiding this comment.
Still not changed.
|
|
||
| MI_metric <-function(simulator){ | ||
| res = predict_testset(simulator,simulator$pool[, 1:14], unfold=TRUE) | ||
| var = res$var |
There was a problem hiding this comment.
var: this intermediate variable is not needed.
There was a problem hiding this comment.
It's still there.
|
|
||
| update_train_pool <- function (simulator,select_index) { | ||
|
|
||
| add = simulator$setting[["add"]] |
There was a problem hiding this comment.
add is not a good name.
There was a problem hiding this comment.
The variable name is still not changed.
No description provided.