Don't know if this of interest, but there's a prob...
# differentialabundance
u
Don't know if this of interest, but there's a problem in the way the digits are rounded (in R scripts, possibly in general). If you look at this line https://github.com/nf-core/modules/blob/master/modules/nf-core/deseq2/differential/templates/deseq2_differential.R#L485 the digits are rounded using round, which means that if you're using 8 digits for example, everything below 1E-8 (or 1E-9) will become 0. Since the default is 4 digits, it means everything below 1E-4. I suggest replacing it with something like
apply(data.frame(comp.results), 2, function(x) sprintf(paste0("%." opt\$round_digits, "g"), x))
This will print everything below significant digits in E format, but still keep a fixed amount of significant digits. I can open an issue, but this will need to be fixed on multiple modules. Would you like me to do that? Maybe this isn't a problem, and should stay the way it is? Should I open an issue on differentialabundane, and then have sub-issues for each module with a PR?
I bumped into this, and I've seen someone complaining about having lots of zeros, so I thought I'd ask
j
That was only intended for test scenarios where we wanted to avoid tiny floating point diffs, don't think it should play a role in production usage.
u
Well, but things like GO enrichment and DESeq p-value become zero, because the default number of digits is 4, and their values are much smaller. Is the default not to round at all when running the pipeline?
My problem that if someone turns the rounding on, a lot of data becomes zeroed out, which is meaningless. In this case, the documentation should be explicit that you should not round unless you're running tests.
Just noticed right now that the default seems to be to round to 4 digits, which will zero a lot of functional enrichment and differentially expressed genes https://github.com/nf-core/differentialabundance/blob/787c8e93e44a476ecc4d4db20903102cc86f03c7/nextflow.config#L27 Is it better to open an issue to have this discussion?
j
Yep, please file an issue. In the last release (the current
master
state), the rounding was ONLY to be used for testing, and for the report, where long floating point things were unhelpful (via the
report_round_digits
option. @Nicolás Alejandro Schcolnicov seems to have changed that in this commit, and @suzannejin in this commit. This should be reverted before release such that any primary results are NOT rounded outside of test scenarios, which should clearly not be happening. CC @Gregor Sturm
u
I've opened an issue and mentioned you. From this comment, does it mean things will be zeroed out in the report? So instead of having a report that the p-value is 4.5E-6, the p-value will be zero?
j
I would expect sig. fig. rounding for display only in the report
u
I don't fully understand that sentence, sorry.
j
so e.g. 4.5898798709879789e-6 -> 4.59e-6, not -> 0
u
If I look at this line of code https://github.com/nf-core/differentialabundance/blob/787c8e93e44a476ecc4d4db20903102cc86f03c7/assets/differentialabundance_report.qmd#L66, it looks like the report does rounding if round_digits is defined. But if it is defined, wouldn't it mean that previous matrices have already been rounded (to zero)? Since the code reads the matrices saved by gprofiler, etc. Am I missing something?
j
Don't worry, we'll make sure to look hard at the rounding before the next release.
u
Okay. Please let me know if you'd like me to review the code, and make a group issue or help in any way on this issue