Add ICmethod "TCSS" - Githubissues

qibaiqi commented 3 years ago

"TCSS" , Topological Clustering Semantic Similarity, measures semantic similarity between GO terms and has a good performance on scoring PPIs.

qibaiqi commented 3 years ago

嗯嗯！好的。我改一下。

---Original--- From: "Guangchuang Yu"<notifications@github.com> Date: Mon, Jan 18, 2021 00:52 AM To: "YuLab-SMU/GOSemSim"<GOSemSim@noreply.github.com>; Cc: "qibaiqi"<1178431277@qq.com>;"Author"<author@noreply.github.com>; Subject: Re: [YuLab-SMU/GOSemSim] Add ICmethod "TCSS" (#35)

@GuangchuangYu commented on this pull request.

@huerqiang 你先review一下。

In R/TCSSMethod.R: > + } + + #get common ancestors + com_anc <- get_common_anc(ID1 = ID1, ID2 = ID2, ont = ont) + + if (length(com_anc) == 0) return(NA) + + #get cluster-ids for each ID + clus1_list <- tcssdata[tcssdata[, "GO"] == ID1, "clusid"] + clus2_list <- tcssdata[tcssdata[, "GO"] == ID2, "clusid"] + #calculate within different clusters + value <- sapply(clus1_list, function(e) { + sapply(clus2_list, get_lca, e, + ID1 = ID1, ID2 = ID2, + tcssdata = tcssdata, com_anc = com_anc, ont = ont) + })
sapply，如果返回是List，用lapply，如果返回是vector，用vapply，不允许使用sapply。

像这样套两层，很难看出来最终返回的是什么。

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

huerqiang commented 3 years ago

好的，老师

GuangchuangYu commented 3 years ago

@huerqiang 发现一点问题写一点，不是等你搞了很久之后，才一起全部放出来。

GuangchuangYu commented 3 years ago

@qibaiqi 还有你的回应完全是不合格的。我的邮件没有回复，github上，只有commit，没有任何文字上的回应和说明。

感觉这么久都白教你了。一定是要写清楚，说清楚，这是在组会上反复强调的东西。而且只有我说话，你一句话不说，这合适？

qibaiqi commented 3 years ago

好的，老师，知道了！这次的 tcssdata的格式在 documentation的地方解释的更详细了一点，另外又新提交了两个修改地方，二强师兄提到需要在其他mclusterSim、mgeneSim等函数的注释行也添加"TCSS"信息，以及删去一些代码上的else，表达更精简一些。 -- 学生郭平凡

------------------ 原始邮件 ------------------ 发件人: "YuLab-SMU/GOSemSim" <notifications@github.com>; 发送时间: 2021年1月20日(星期三) 晚上9:47 收件人: "YuLab-SMU/GOSemSim"<GOSemSim@noreply.github.com>; 抄送: "guopingfan"<1178431277@qq.com>;"Mention"<mention@noreply.github.com>; 主题: Re: [YuLab-SMU/GOSemSim] Add ICmethod "TCSS" (#35)

@qibaiqi 还有你的回应完全是不合格的。我的邮件没有回复，github上，只有commit，没有任何文字上的回应和说明。

感觉这么久都白教你了。一定是要写清楚，说清楚，这是在组会上反复强调的东西。

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

GuangchuangYu commented 3 years ago

你的回答已经是无视我的话，然后自说自话了。

qibaiqi commented 3 years ago

老师，我不懂，难道是要再定义一个函数，检查下testdata的格式？还是说提前准备好个模板数据存到包里？

------------------ 原始邮件 ------------------ 发件人: "YuLab-SMU/GOSemSim" <notifications@github.com>; 发送时间: 2021年1月20日(星期三) 晚上10:56 收件人: "YuLab-SMU/GOSemSim"<GOSemSim@noreply.github.com>; 抄送: "guopingfan"<1178431277@qq.com>;"Mention"<mention@noreply.github.com>; 主题: Re: [YuLab-SMU/GOSemSim] Add ICmethod "TCSS" (#35)

你的回答已经是无视我的话，然后自说自话了。

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

GuangchuangYu commented 3 years ago

还是在自说自话，你整天都心不在焉，一天混过一天。

你写一个函数，我问你某个参数到底是什么？这样的话，你不能回答？而且是我连问了三次！

不要再扯别的，也不要再问我问题，仿佛好像我没回答你在等回答似的，惯得你都不看问题，不经过大脑。你就回答到底你要求的是什么样的数据做为输入就行。

GuangchuangYu commented 3 years ago

所以我说了这么多，还是我不问，就没有反馈？

几天前二强就应该跟你讲过这个问题了，就应该有commit上来。

在这个PR没接merge之前，都要不断搞，争取早日被merge。

做事这么不主动，是不行的。这所以放了5天，第一我不可能天天盯着你，第二我认为组会上说的，你应当有所反思。

GuangchuangYu commented 3 years ago

@huerqiang 在这个PR里没有一句发言，搞到你好像是个ghost一样。你私下讲的东西，要summarize发上来。事情要跟进，我等着看你们搞到跟我说OK了，我再看。

qibaiqi commented 3 years ago

老师，关于testdata的解释之前已经commit了，https://github.com/qibaiqi/GOSemSim/commit/c1d842622e0f2c30720a47532d9ac8dca1b08c2c，R/TCSScutoff.R，在这里解释了 testdata必须是data.frame，有三列，列名必须是"pro1", "pro2", "label", 规定了每列的属性，character, character, logical. 最近这几天改的地方比较小，本想要等有大的明显的改动再commit上去。以后的commit会另写文字详细解释，跟二强师兄的交流也会从地下转地上。

------------------ 原始邮件 ------------------ 发件人: "YuLab-SMU/GOSemSim" <notifications@github.com>; 发送时间: 2021年1月26日(星期二) 下午5:25 收件人: "YuLab-SMU/GOSemSim"<GOSemSim@noreply.github.com>; 抄送: "guopingfan"<1178431277@qq.com>;"Mention"<mention@noreply.github.com>; 主题: Re: [YuLab-SMU/GOSemSim] Add ICmethod "TCSS" (#35)

所以我说了这么多，还是我不问，就没有反馈？

几天前二强就应该跟你讲过这个问题了，就应该有commit上来。

在这个PR没接merge之前，都要不断搞，争取早日被merge。

做事这么不主动，是不行的。这所以放了5天，第一我不可能天天盯着你，第二我认为组会上说的，你应当有所反思。

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

qibaiqi commented 3 years ago

老师，师兄，这次的提交是对代码里的一些过于啰嗦的代码进行更改，主要包括：

else 语句的删去，if 语句后加了条件，满足条件返回 a 值，不满足条件进入 else 返回 b 值，此种情况下，else 语句可以删去，当不满足条件运行到下一句时直接返回b值。有 R/TCSSMethod.R 61-66行，126-133行，R/processTCSS.R 109-113行；

在 R/processTCSS.R中，参数 geneAnno没有用到，是之前旧代码遗留没有及时删去的，此次在定义和调用函数的地方均删去；

在 R/TCSScutoff.R 中，90, 91行用了 vapply(testdata$pro1, function(e) e %in% all_pro, logical(1)), 直接用 %in%判断就行，所以删去vapply；94行用了testdata_in[!duplicated(testdata_in), ]，直接用unique就行，此处也改正；

在R/processTCSS.R中，需要计算每个sub-graph的最大IC值 meta_maxIC 作为分母，此处在主函数里占了三行，就把内容放到"副"函数里，主函数里一句话就可以了。以上前三条是二强师兄指导。

------------------ 原始邮件 ------------------ 发件人: "YuLab-SMU/GOSemSim" <notifications@github.com>; 发送时间: 2021年1月26日(星期二) 晚上6:54 收件人: "YuLab-SMU/GOSemSim"<GOSemSim@noreply.github.com>; 抄送: "guopingfan"<1178431277@qq.com>;"Mention"<mention@noreply.github.com>; 主题: Re: [YuLab-SMU/GOSemSim] Add ICmethod "TCSS" (#35)

@huerqiang commented on this pull request.

In R/processTCSS.R: > @@ -0,0 +1,170 @@ +#' Title prepare tcss data for TCSS to calculate semantic similarity +#' +#' @param ont ontology +#' @param geneAnno the annotation for genes or gene products +#' @param IC information content +#' @param cutoff the topology cutoff +#' +#' @return data.frame +#' @noRd +process_tcss <- function(ont, geneAnno, IC, cutoff = NULL) { +
第四处修改：这里的geneAnno参数并没有在函数里用到。

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

qibaiqi commented 3 years ago

老师，师兄，这次的提交主要包含三方面：

还是else结构的删去，if后面直接return，上次检查同类型错误时遗漏的地方，by二强师兄；
看到老师的代码里在遍历计算时，是先构建matrix，再用mapply，此处在R/TCSSMethod.R tcssMethod_internal函数里，删去了lapply套lapply的部分，改为了构建matrix和用mapply；
其他的一些小问题，注释英文单词拼写错误等。 https://github.com/qibaiqi/GOSemSim/commit/c1cf52c8e979720f4d1bfbdc020e4ba0003a60a2

------------------ 原始邮件 ------------------ 发件人: "YuLab-SMU/GOSemSim" <notifications@github.com>; 发送时间: 2021年1月29日(星期五) 下午4:20 收件人: "YuLab-SMU/GOSemSim"<GOSemSim@noreply.github.com>; 抄送: "guopingfan"<1178431277@qq.com>;"Mention"<mention@noreply.github.com>; 主题: Re: [YuLab-SMU/GOSemSim] Add ICmethod "TCSS" (#35)

@huerqiang commented on this pull request.

In R/TCSScutoff.R: > @@ -184,7 +184,7 @@ decide_cutoff <- function(auc_F1_score, cutoffs) { loca <- which(auc_mutiply_F1 == max(auc_mutiply_F1)) if (length(loca) == 1) { - cutoff <- cutoffs[loca] + return(cutoffs[loca]) } else {
这里if语句中已经有return了，那么再写个else就多余了。

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

qibaiqi commented 3 years ago

老师，师兄，这次提交内容比较少，主要是：前面要求 test-set 里面的 label 必须含有 TRUE 和 FALSE，是因为在计算 auc 和 F1-score 时要求必须有正负测试集。为了避免在后面计算的时候报错，想要在前面筛选测试集的时候就提示一下用户，一来节省时间，二来避免用户不理解报错信息，不知道卡在哪里。就添加了一条判断语句，如果判断成立就打印一条报错信息。

------------------ 原始邮件 ------------------ 发件人: "YuLab-SMU/GOSemSim" <notifications@github.com>; 发送时间: 2021年1月29日(星期五) 下午4:20 收件人: "YuLab-SMU/GOSemSim"<GOSemSim@noreply.github.com>; 抄送: "guopingfan"<1178431277@qq.com>;"Mention"<mention@noreply.github.com>; 主题: Re: [YuLab-SMU/GOSemSim] Add ICmethod "TCSS" (#35)

@huerqiang commented on this pull request.

In R/TCSScutoff.R: > @@ -184,7 +184,7 @@ decide_cutoff <- function(auc_F1_score, cutoffs) { loca <- which(auc_mutiply_F1 == max(auc_mutiply_F1)) if (length(loca) == 1) { - cutoff <- cutoffs[loca] + return(cutoffs[loca]) } else {
这里if语句中已经有return了，那么再写个else就多余了。

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

huerqiang commented 3 years ago

@GuangchuangYu 老师，平凡的代码我感觉现在写的可以了，我没发现别的问题了。不过您的代码是将计算过程写成Rcpp::NumericMatrix infoContentMethod_cpp来加快运行速度，而平凡的代码是纯粹R语言的，我目前对Rcpp的使用还不熟悉，所以没往这方面仔细看。

qibaiqi commented 3 years ago

老师，关于您说到的几点：

Title 已经去掉，get_XX函数名、testdata改为ppidata都已经改了，参数说明已经加了；
在TCSScutoff.R文件里，一共有5个函数，tcss_cutoff 函数调用了其他四个函数，其他四个函数是并列，彼此之间没有调用：
ppidata(原testdata)要求的格式： 3.1 “label” 那一列中用 TRUE/FALSE, 或者 0/1, 或者"yes"/"no", 都无所谓，确实只是个符号，中间只是table计算下频率，看看是否缺失，各自的个数是多少，但也因为这样，所以要指定，必须指定符号，不然我感觉没法匹配。 3.2 当然没有必要指定列名，是我为了我自己取值方便加的条件，只要知道哪一列是蛋白名，哪一列是label，就可以往下计算。

但是，如果不指定列名 "pro1", "pro2", "label" 的话，就是按位置取值，ppidata[,1], ppidata[,2], ppidata[,3], 这样的话没法确定里面的内容感觉会更危险，二强师兄提出加一条判断语句，如果列名不是pro1, pro2, label，就用位置取, 老师，这样改可以吗？

------------------ 原始邮件 ------------------ 发件人: "YuLab-SMU/GOSemSim" <notifications@github.com>; 发送时间: 2021年2月4日(星期四) 下午3:46 收件人: "YuLab-SMU/GOSemSim"<GOSemSim@noreply.github.com>; 抄送: "guopingfan"<1178431277@qq.com>;"Mention"<mention@noreply.github.com>; 主题: Re: [YuLab-SMU/GOSemSim] Add ICmethod "TCSS" (#35)

@GuangchuangYu commented on this pull request.

In R/TCSSMethod.R: > + rep(clus2_list, each = length(clus1_list)), + MoreArgs = list(ID1 = ID1, ID2 = ID2, + tcssdata = tcssdata, com_anc = com_anc, ont = ont)), + dimnames = list(clus1_list, clus2_list), + ncol = length(clus2_list)) + + value <- na.omit(unlist(value)) + value <- value[value != Inf & value != -Inf] + + if (is.null(value) || length(value) == 0) return(NULL) + #here max value means lowest common ancestor + max(value) +} + +#' Title get lowest common ancestors's value +#'
Title去掉。

In R/TCSSMethod.R: > @@ -0,0 +1,132 @@ +#' Title Method TCSS for semantic similarity measuring +#'
Title去掉

In R/TCSSMethod.R: > +#' @examples +#' library(org.Hs.eg.db) +#' semdata <- godata(org.Hs.eg.db, "ENTREZID", "MF", computeIC = TRUE, +#' processTCSS = TRUE, cutoff = NULL) +#' termSim("GO:0000003", "GO:0009987", semdata, method = "TCSS") +#process two term vectors +tcssMethod <- function(t1, t2, semData) { + matrix(mapply(tcssMethod_internal, + rep(t1, length(t2)), + rep(t2, each = length(t1)), + MoreArgs = list(semData = semData)), + dimnames = list(t1, t2), ncol = length(t2)) +} + +#' Title process one term with one term +#'
又有Title，这里模板写个title，是让你写个title。你留着这个字多余。

In R/TCSScutoff.R: > +#' loca_1 <- sample(len, s_len, replace = T) +#' #negative set +#' loca_2 <- sample(len, s_len, replace = T) +#' loca_3 <- sample(len, s_len, replace = T) +#' +#' #union as testdata +#' testdata <- data.frame(pro1 = c(ppi$from[loca_1], ppi$from[loca_2]), +#' pro2 = c(ppi$to[loca_1], ppi$to[loca_3]), +#' label = c(rep(TRUE, s_len), rep(FALSE, s_len)), +#' stringsAsFactors = FALSE) +#' +#' cutoff <- get_cutoff(OrgDb = org.Hs.eg.db, keytype = "ENSEMBLPROT", +#' ont = "BP", combine_method = "max", IEAdrop = FALSE, testdata) +#' } +get_cutoff <- function(OrgDb = NULL, keytype = "ENTREZID", ont, + combine_method = "max", IEAdrop = FALSE, testdata) {
函数名和变量名，要调整一下。get_什么好像是要去访问拿到点什么之类的。一般我们在OO里，访问对象的数据就写get_xx函数。你这个方法叫tcss，函数名可以写tcss_cutoff，后面参数testdata，也是指意不明确的，可以改为ppi_data

In R/TCSScutoff.R: > +#' @return data.frame +#' @noRd +get_test_set <- function(all_pro, testdata) { + #remove those proteins that have zero annotations + len1 <- testdata[, "pro1"] %in% all_pro + len2 <- testdata[, "pro2"] %in% all_pro + testdata_in <- testdata[len1 & len2, ] + #data de-duplication + test_set <- unique(testdata_in) + + if (dim(test_set)[1] == 0) { + stop("the length of test set is 0, none items have GO annotation") + } + + if (!all(c("TRUE","FALSE") %in% test_set[, "label"])) { + stop("the label in test set must contain TRUE and FALSE")
逻辑变量TRUE， FALSE是拿来当字符串用的？

In R/TCSScutoff.R: > + + if (!all(c("TRUE","FALSE") %in% test_set[, "label"])) { + stop("the label in test set must contain TRUE and FALSE") + } + + freq <- table(test_set[, "label"]) + + message(paste("positive set's length is", freq["TRUE"], + ", negative set's length is", freq["FALSE"])) + + return(test_set) +} + +#' Title compute prediction value +#' +#' @param test_set data.frame
参数的说明要详细一点。

In R/TCSScutoff.R: > +#' @noRd +#' +computePre <- function(test_set, OrgDb, keytype, ont, cutoff, + combine_method, IEAdrop) { + #different cutoffs have different semdata + suppressMessages(semdata <- godata(OrgDb = OrgDb, keytype = keytype, + ont = ont, computeIC = TRUE, + processTCSS = TRUE, cutoff = cutoff)) + #similarity value is calculated under this cutoff + mapply(function(e, f) geneSim(e, f, + semData = semdata, + measure = "TCSS", + combine = combine_method, + drop = IEAdrop), + test_set[, "pro1"], test_set[, "pro2"]) +}
这里就表明，你对test_set是有要求的，column name必须是"pro1"和“pro2"，有这个必要性？

In R/TCSScutoff.R: > + loca <- which(auc_mutiply_F1 == max(auc_mutiply_F1)) + + if (length(loca) == 1) return(cutoffs[loca]) + + #if not only one pair of auc and F1-score have same product + #take the one with larger auc + s_auc <- auc_F1_score[loca, "auc"] + + auc_loca <- which(s_auc == max(s_auc)) + + if (length(auc_loca) == 1) return(cutoffs[loca[auc_loca]]) + + #if more than one pair of auc and F1-score are both same + #take the smaller cutoff for time saving + return(cutoffs[loca[min(auc_loca)]]) +}
这个decide_cutoff和上面的get_cutoff什么关系？你给我画一个函数的关系图，让我明白函数之间的调用关系和内在的逻辑。

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

GuangchuangYu commented 3 years ago

第二点，我让你画图，你手画也可以。你所有的函数的关系图。第三点，不管你是01还是yn，还是别的什么，做为二元变量，统一为TF,你的代码完全按照逻辑变量去写。你只要强制用户输入就行。对于ppidata，你也是强制定义，第一个column，第二个column都是字符串型，第三个column是逻辑型，只能三个column，完事。

qibaiqi commented 3 years ago

老师，师兄，这次提交的内容主要有：

重新定义 ppidata 的格式要求 : 一个包含正集和负集的数据框，正集是已经验证为PPI的蛋白对。ppidata只有三列，第一，二列为字符串，第三列为逻辑值，TRUE/FALSE.
把代码里的列名也删掉；
增加一条判断 ppidata 三列数据类型的语句，若有一个不通过，则报错停止运行；
在用 ROCR 包的函数计算 auc 和 F1-score 时，添上了 label.ordering = c(FALSE, TRUE)，指定负集的标签是 FALSE, 正集的标签是 TRUE，更安全。

https://github.com/qibaiqi/GOSemSim/commit/5a5a00ac9809df3cd01bb549117c834e47d09de5

------------------ 原始邮件 ------------------ 发件人: "YuLab-SMU/GOSemSim" <notifications@github.com>; 发送时间: 2021年2月4日(星期四) 晚上9:05 收件人: "YuLab-SMU/GOSemSim"<GOSemSim@noreply.github.com>; 抄送: "guopingfan"<1178431277@qq.com>;"Mention"<mention@noreply.github.com>; 主题: Re: [YuLab-SMU/GOSemSim] Add ICmethod "TCSS" (#35)

第二点，我让你画图，你手画也可以。你所有的函数的关系图。第三点，不管你是01还是yn，还是别的什么，做为二元变量，统一为TF,你的代码完全按照逻辑变量去写。你只要强制用户输入就行。对于ppidata，你也是强制定义，第一个column，第二个column都是字符串型，第三个column是逻辑型，只能三个column，完事。

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

GuangchuangYu commented 3 years ago

有些test_set改ppidata，有些没改。不是要我每一处都一一指出，然后我指哪才改哪的。说了两处名字，就好好查一遍名字。

另外你根本不懂逻辑变量。

> x = c(TRUE, FALSE, TRUE)
> microbenchmark::microbenchmark(table = table(x), sum=c(sum(x), sum(!x)))
Unit: nanoseconds
  expr   min      lq     mean  median      uq    max neval cld
 table 52947 53504.5 55103.09 53838.0 54470.0 133722   100   b
   sum   443   507.5   786.36   813.5   909.5   6939   100  a

另外还有图没画。

qibaiqi commented 3 years ago

老师，这次修改了一些定义模糊的变量名，包括把 test_set 改为 filtered_ppidata(因为 test_set 就是删去注释为空的蛋白后的 ppidata), value 改为 sim_value, ppidata_in 改为 ppidata_exist, s_auc 改为 select_auc; 还改了 1. 把(!a | !b |!c) 合并成 !(a & b & c) ; 2. 在 TCSSMethod.R里，把if语句和else语句里重复的语句提出来，放到外面，精简一下。新画的函数之间调用关系的图整合成docx文档另发邮件给您。

还有，老师，我懂了，逻辑变量可以直接进行算术，除了NA，TRUE/FALSE 可以直接当做 1/0来用.

https://github.com/qibaiqi/GOSemSim/commit/2c667fc6ac99da0509781ae417cdb74817358d84

------------------ 原始邮件 ------------------ 发件人: "YuLab-SMU/GOSemSim" <notifications@github.com>; 发送时间: 2021年2月5日(星期五) 下午3:24 收件人: "YuLab-SMU/GOSemSim"<GOSemSim@noreply.github.com>; 抄送: "guopingfan"<1178431277@qq.com>;"Mention"<mention@noreply.github.com>; 主题: Re: [YuLab-SMU/GOSemSim] Add ICmethod "TCSS" (#35)

有些test_set改ppidata，有些没改。不是要我每一处都一一指出，然后我指哪才改哪的。说了两处名字，就好好查一遍名字。

另外你根本不懂逻辑变量。 > x = c(TRUE, FALSE, TRUE) > microbenchmark::microbenchmark(table = table(x), sum=c(sum(x), sum(!x))) Unit: nanoseconds expr min lq mean median uq max neval cld table 52947 53504.5 55103.09 53838.0 54470.0 133722 100 b sum 443 507.5 786.36 813.5 909.5 6939 100 a
另外还有图没画。

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

qibaiqi commented 3 years ago

老师， max(ICT)即cutoff可能取值范围的上限，一般在4.0左右。ICT是只针对GO ontology拓扑结构的值，公式是该term的后代数目除以总的节点数目，再取-log(), 因此是表明该term在ontology层次高低的意思，从根节点到叶节点，ICT越来越大。所以和物种注释信息没有关系。然后，作者在原文中说，子集选取时是选取ICT值小于cutoff的，所以我认为cutoff的可能取值范围就是ICT的值域，不过不能取0，就从0.1开始算， cutoffs <- seq(0.1, max(ICT), 0.1). 用逻辑值判断是否存在的地方已经改了，之前当做符号来用，现在改掉，用all()来直接判断。 https://github.com/qibaiqi/GOSemSim/commit/4bf3ec11a99807fec302ecd506201c176f6598a5

------------------ 原始邮件 ------------------ 发件人: "YuLab-SMU/GOSemSim" <notifications@github.com>; 发送时间: 2021年2月6日(星期六) 晚上11:03 收件人: "YuLab-SMU/GOSemSim"<GOSemSim@noreply.github.com>; 抄送: "guopingfan"<1178431277@qq.com>;"Mention"<mention@noreply.github.com>; 主题: Re: [YuLab-SMU/GOSemSim] Add ICmethod "TCSS" (#35)

@GuangchuangYu commented on this pull request.

In R/TCSScutoff.R: > + if (!(is.character(ppidata[, 1]) & is.character(ppidata[, 2]) & + is.logical(ppidata[, 3]))) { + stop("ppidata must be a data.frame with three columns:character, character, logical") + } + + #remove those proteins that have zero annotations + len1 <- ppidata[, 1] %in% all_pro + len2 <- ppidata[, 2] %in% all_pro + ppidata_exist <- ppidata[len1 & len2, ] + filtered_ppidata <- unique(ppidata_exist) + + if (dim(filtered_ppidata)[1] == 0) { + stop("the length of filtered ppidata is 0, none items have GO annotation") + } + + if (!all(c(TRUE, FALSE) %in% filtered_ppidata[, 3])) {
91行会用is.logical，这里又不会用了。

In R/TCSScutoff.R: > +#' } +tcss_cutoff <- function(OrgDb = NULL, keytype = "ENTREZID", ont, + combine_method = "max", IEAdrop = FALSE, ppidata) { + + anno_data <- godata(OrgDb, keytype = keytype, ont = ont, computeIC = TRUE, + processTCSS = FALSE, cutoff = NULL) + #cutoff is in the range of ICT value + GO <- unique(names(anno_data@IC)) + offspring <- switch(ont, + MF = AnnotationDbi::as.list(GOMFOFFSPRING), + BP = AnnotationDbi::as.list(GOBPOFFSPRING), + CC = AnnotationDbi::as.list(GOCCOFFSPRING)) + #compute ICT value for each term + ICT <- computeICT(GO, offspring) + #cutoffs, all possible cutoff values + cutoffs <- seq(0.1, max(ICT), 0.1)
max(ICT)是个什么样的取值？换个物种呢？

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

qibaiqi commented 3 years ago

老师，师兄，本次修改内容主要有：

为了不重复计算semdata, 将godata操作提到循环外面，只处理一次orgdb, keytype信息，只计算一次IC值。对于不同的cutoffs，只计算tcssdata。并将更新的tcssdata放入semdata中；
更改stop里的报错信息，使用户更好理解报错原因及解决办法；
更改一些注释信息；
缩小cutoffs的取值范围。即对于部分cutoffs，对应的tcssdata一致，这部分cutoffs可以只留下其中一个，避免无效计算，节省时间；
逻辑判断时用&&和|| 。

https://github.com/YuLab-SMU/GOSemSim/pull/35/commits/726a83fd1c61fe928ed7b890fa3928d0e85d8745

------------------ 原始邮件 ------------------ 发件人: "YuLab-SMU/GOSemSim" <notifications@github.com>; 发送时间: 2021年2月6日(星期六) 晚上11:03 收件人: "YuLab-SMU/GOSemSim"<GOSemSim@noreply.github.com>; 抄送: "guopingfan"<1178431277@qq.com>;"Mention"<mention@noreply.github.com>; 主题: Re: [YuLab-SMU/GOSemSim] Add ICmethod "TCSS" (#35)

@GuangchuangYu commented on this pull request.

In R/TCSScutoff.R: > + if (!(is.character(ppidata[, 1]) & is.character(ppidata[, 2]) & + is.logical(ppidata[, 3]))) { + stop("ppidata must be a data.frame with three columns:character, character, logical") + } + + #remove those proteins that have zero annotations + len1 <- ppidata[, 1] %in% all_pro + len2 <- ppidata[, 2] %in% all_pro + ppidata_exist <- ppidata[len1 & len2, ] + filtered_ppidata <- unique(ppidata_exist) + + if (dim(filtered_ppidata)[1] == 0) { + stop("the length of filtered ppidata is 0, none items have GO annotation") + } + + if (!all(c(TRUE, FALSE) %in% filtered_ppidata[, 3])) {
91行会用is.logical，这里又不会用了。

In R/TCSScutoff.R: > +#' } +tcss_cutoff <- function(OrgDb = NULL, keytype = "ENTREZID", ont, + combine_method = "max", IEAdrop = FALSE, ppidata) { + + anno_data <- godata(OrgDb, keytype = keytype, ont = ont, computeIC = TRUE, + processTCSS = FALSE, cutoff = NULL) + #cutoff is in the range of ICT value + GO <- unique(names(anno_data@IC)) + offspring <- switch(ont, + MF = AnnotationDbi::as.list(GOMFOFFSPRING), + BP = AnnotationDbi::as.list(GOBPOFFSPRING), + CC = AnnotationDbi::as.list(GOCCOFFSPRING)) + #compute ICT value for each term + ICT <- computeICT(GO, offspring) + #cutoffs, all possible cutoff values + cutoffs <- seq(0.1, max(ICT), 0.1)
max(ICT)是个什么样的取值？换个物种呢？

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

qibaiqi commented 3 years ago

老师，师兄，这次提交了两次修改：

把重复写的一段 filtered_ppidata[, 3] 提出来，用len来代表其长度，number_T, number_F分别代表TRUE/FALSE的个数； https://github.com/YuLab-SMU/GOSemSim/pull/35/commits/67c73ade61977798e6e5fda9335622ad71122e0f
我回去后又分析了一遍ICT的计算方式，计算的过程，总结了一下，并且在PPT里简单说明这次更改的原因，以及更改后ICT在不同物种下的情况。 https://github.com/YuLab-SMU/GOSemSim/pull/35/commits/7f8f2269d90459da2134a01f265997bbecf0728e

------------------ 原始邮件 ------------------ 发件人: "guopingfan" <1178431277@qq.com>; 发送时间: 2021年3月1日(星期一) 凌晨0:21 收件人: "YuLab-SMU/GOSemSim"<reply@reply.github.com>;

主题: 回复： [YuLab-SMU/GOSemSim] Add ICmethod "TCSS" (#35)

老师，师兄，本次修改内容主要有：

为了不重复计算semdata, 将godata操作提到循环外面，只处理一次orgdb, keytype信息，只计算一次IC值。对于不同的cutoffs，只计算tcssdata。并将更新的tcssdata放入semdata中；
更改stop里的报错信息，使用户更好理解报错原因及解决办法；
更改一些注释信息；
缩小cutoffs的取值范围。即对于部分cutoffs，对应的tcssdata一致，这部分cutoffs可以只留下其中一个，避免无效计算，节省时间；
逻辑判断时用&&和|| 。

https://github.com/YuLab-SMU/GOSemSim/pull/35/commits/726a83fd1c61fe928ed7b890fa3928d0e85d8745

------------------ 原始邮件 ------------------ 发件人: "YuLab-SMU/GOSemSim" <notifications@github.com>; 发送时间: 2021年2月6日(星期六) 晚上11:03 收件人: "YuLab-SMU/GOSemSim"<GOSemSim@noreply.github.com>; 抄送: "guopingfan"<1178431277@qq.com>;"Mention"<mention@noreply.github.com>; 主题: Re: [YuLab-SMU/GOSemSim] Add ICmethod "TCSS" (#35)

@GuangchuangYu commented on this pull request.

In R/TCSScutoff.R: > + if (!(is.character(ppidata[, 1]) & is.character(ppidata[, 2]) & + is.logical(ppidata[, 3]))) { + stop("ppidata must be a data.frame with three columns:character, character, logical") + } + + #remove those proteins that have zero annotations + len1 <- ppidata[, 1] %in% all_pro + len2 <- ppidata[, 2] %in% all_pro + ppidata_exist <- ppidata[len1 & len2, ] + filtered_ppidata <- unique(ppidata_exist) + + if (dim(filtered_ppidata)[1] == 0) { + stop("the length of filtered ppidata is 0, none items have GO annotation") + } + + if (!all(c(TRUE, FALSE) %in% filtered_ppidata[, 3])) {
91行会用is.logical，这里又不会用了。

In R/TCSScutoff.R: > +#' } +tcss_cutoff <- function(OrgDb = NULL, keytype = "ENTREZID", ont, + combine_method = "max", IEAdrop = FALSE, ppidata) { + + anno_data <- godata(OrgDb, keytype = keytype, ont = ont, computeIC = TRUE, + processTCSS = FALSE, cutoff = NULL) + #cutoff is in the range of ICT value + GO <- unique(names(anno_data@IC)) + offspring <- switch(ont, + MF = AnnotationDbi::as.list(GOMFOFFSPRING), + BP = AnnotationDbi::as.list(GOBPOFFSPRING), + CC = AnnotationDbi::as.list(GOCCOFFSPRING)) + #compute ICT value for each term + ICT <- computeICT(GO, offspring) + #cutoffs, all possible cutoff values + cutoffs <- seq(0.1, max(ICT), 0.1)
max(ICT)是个什么样的取值？换个物种呢？

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

GuangchuangYu commented 3 years ago

你当前首要的任务是确保ICT的计算是正确的，速度是其次的。

qibaiqi commented 3 years ago

嗯嗯，好的。

------------------ 原始邮件 ------------------ 发件人: "YuLab-SMU/GOSemSim" <notifications@github.com>; 发送时间: 2021年3月5日(星期五) 中午11:32 收件人: "YuLab-SMU/GOSemSim"<GOSemSim@noreply.github.com>; 抄送: "guopingfan"<1178431277@qq.com>;"Mention"<mention@noreply.github.com>; 主题: Re: [YuLab-SMU/GOSemSim] Add ICmethod "TCSS" (#35)

你当前首要的任务是确保ICT的计算是正确的，速度是其次的。

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

qibaiqi commented 3 years ago

老师，本次离上次发邮件之间隔了好几个commit，主要是中间改来改去，有些地方改的有点频繁，觉得还是放到一起说比较清晰一点。主要改的地方有：

定义环境 .ancCache存储节点的祖先节点信息，方便检索，加快速度； https://github.com/YuLab-SMU/GOSemSim/pull/35/commits/0cd0b3820ffa38304085bd240162a8bbf7930db4
当判断一个值是否存在时，用 any(x == b)比 %in% 要快一点，把存在这种情况的地方改用 %in%(已测过速); https://github.com/YuLab-SMU/GOSemSim/pull/35/commits/00e6d55371de67f832e9f54181ff2fdbdd669ca2
改参数，函数命名，number_T 改为 nTrue, loca 改为 pos; https://github.com/YuLab-SMU/GOSemSim/pull/35/commits/6304c3f332e92864078f507472ae33e69e7ce686
改ICT计算方式，分子分母都应为有注释的节点； https://github.com/YuLab-SMU/GOSemSim/pull/35/commits/e27acdfb6f973794d4e30d68346c60a4ee3d32cf
计算cutoff时，去掉IEA这个参数，经计算证明发现有无IEA没有太大影响，并且在原作者文献中也没有提到； https://github.com/YuLab-SMU/GOSemSim/pull/35/commits/d5e594c188d90125308aaffc0d92f713f8495c83
重新写tcss_cutoff函数，先用大范围计算，再小范围计算； https://github.com/YuLab-SMU/GOSemSim/pull/35/commits/d3dfe322e777a7c1ac61c60eced847eedb465ae6

谢谢老师！

------------------ 原始邮件 ------------------ 发件人: "guopingfan" @.>; 发送时间: 2021年3月5日(星期五) 中午11:37 @.>;

主题: 回复： [YuLab-SMU/GOSemSim] Add ICmethod "TCSS" (#35)

嗯嗯，好的。

------------------ 原始邮件 ------------------ 发件人: "YuLab-SMU/GOSemSim" @.>; 发送时间: 2021年3月5日(星期五) 中午11:32 @.>; @.**@.>; 主题: Re: [YuLab-SMU/GOSemSim] Add ICmethod "TCSS" (#35)

你当前首要的任务是确保ICT的计算是正确的，速度是其次的。

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

qibaiqi commented 3 years ago

老师，师兄，这次修改改了两个方面：

在计算ICT的函数里，加上了必要的花括号，以及把重复计算的变量定义为num提到外面；
用slot()提取对象的字段，删去@。谢谢！

------------------ 原始邮件 ------------------ 发件人: "guopingfan" @.>; 发送时间: 2021年3月18日(星期四) 中午1:55 @.>;

主题: 回复： [YuLab-SMU/GOSemSim] Add ICmethod "TCSS" (#35)

老师，本次离上次发邮件之间隔了好几个commit，主要是中间改来改去，有些地方改的有点频繁，觉得还是放到一起说比较清晰一点。主要改的地方有：

定义环境 .ancCache存储节点的祖先节点信息，方便检索，加快速度； https://github.com/YuLab-SMU/GOSemSim/pull/35/commits/0cd0b3820ffa38304085bd240162a8bbf7930db4
当判断一个值是否存在时，用 any(x == b)比 %in% 要快一点，把存在这种情况的地方改用 %in%(已测过速); https://github.com/YuLab-SMU/GOSemSim/pull/35/commits/00e6d55371de67f832e9f54181ff2fdbdd669ca2
改参数，函数命名，number_T 改为 nTrue, loca 改为 pos; https://github.com/YuLab-SMU/GOSemSim/pull/35/commits/6304c3f332e92864078f507472ae33e69e7ce686
改ICT计算方式，分子分母都应为有注释的节点； https://github.com/YuLab-SMU/GOSemSim/pull/35/commits/e27acdfb6f973794d4e30d68346c60a4ee3d32cf
计算cutoff时，去掉IEA这个参数，经计算证明发现有无IEA没有太大影响，并且在原作者文献中也没有提到； https://github.com/YuLab-SMU/GOSemSim/pull/35/commits/d5e594c188d90125308aaffc0d92f713f8495c83
重新写tcss_cutoff函数，先用大范围计算，再小范围计算； https://github.com/YuLab-SMU/GOSemSim/pull/35/commits/d3dfe322e777a7c1ac61c60eced847eedb465ae6

谢谢老师！

------------------ 原始邮件 ------------------ 发件人: "guopingfan" @.>; 发送时间: 2021年3月5日(星期五) 中午11:37 @.>;

主题: 回复： [YuLab-SMU/GOSemSim] Add ICmethod "TCSS" (#35)

嗯嗯，好的。

------------------ 原始邮件 ------------------ 发件人: "YuLab-SMU/GOSemSim" @.>; 发送时间: 2021年3月5日(星期五) 中午11:32 @.>; @.**@.>; 主题: Re: [YuLab-SMU/GOSemSim] Add ICmethod "TCSS" (#35)

你当前首要的任务是确保ICT的计算是正确的，速度是其次的。

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

qibaiqi commented 3 years ago

老师，师兄，此次更改主要：

把tcssdata改为list，list内部包含一个data.frame和一个list，分别为该节点的内部元素和对应ica值，该节点所属的cluster。
把TCSScutoff.R 又改回原来的样子，即对所有可能的cutoff计算，而不是先大范围再小范围。因为感觉自己写的不好，先用原来的方法计算，等想到好的方法写了，再改。

qibaiqi commented 3 years ago

自己在计算途中产生了一个报错，直接原因为：当ID不存在时，所得的sim_value为空，原来的TCSS方法返回NULL。当有多个值时，如同时包含NA，NULL，实数，matrix会转为list, 因此combine_method在计算的时候报错，(max(SimScores, na.rm=TRUE)). 因此，针对这种情况，改正tcssMethod，改为在sim_value为空的情况下也返回NA，表明暂无该节点的相关信息，为未知，也更符合常理。最终，相似值只有两种情况，实数或NA。

qibaiqi commented 3 years ago

针对上一条comment作出改正，不是matrix转为list，而是mapply()里的默认参数SIMPLIFY=TRUE，会在部分情况下输出为vector或matrix，当遇到c(NA, NULL)时，mapply输出的是一个个list，因此在求最大值时报错。因此，按照老师的指导，限制mapply的输出为list，即将参数SIMPLIFY设置为FALSE。再加上unlist函数，便于后续用combine methods求值。

qibaiqi commented 3 years ago

考虑各种输入可能性，确定了tcssMethod_internal函数的输出只有实数或NA。因此，在tcssMethod函数里可以建立matrix。有感于此，对TCSSMethod.R整个进行了每一步输出结果的探索，做了以下修改：

删除了对sim_value的筛选，sim_value[!is.na(sim_value) & !is.infinite(sim_value)] ，发现没有理由且没有必要；
对match函数加入nomatch=0是为了避免引入不合理的NA；
在create_sub_terms函数中的修改，不仅去掉其他子集内的节点，同时也要去掉其他子集的根节点，是为了保证只有meta_terms的所属cluster为本身+"meta"。

qibaiqi commented 3 years ago

修改有：

把tcssdata改为list嵌套list结构，不用之前list嵌套data.frame的结构，更方便直观，速度上的提升非常微小；
去除掉多余的is.na的判断，没有必要，经检验，没有产生NA的可能；
改掉一些变量的名字。

YuLab-SMU / GOSemSim

Add ICmethod "TCSS" #35