Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error: invalid 'xscale' in viewport #800

Closed
bersbersbers opened this issue Sep 8, 2021 · 19 comments
Closed

Error: invalid 'xscale' in viewport #800

bersbersbers opened this issue Sep 8, 2021 · 19 comments

Comments

@bersbersbers
Copy link

ComplexHeatmap::Heatmap(matrix(c(0, 0, 1, 1), nrow=2))

gives

Error in valid.viewport(x, y, width, height, just, gp, clip, mask, xscale,  :
  invalid 'xscale' in viewport
@bersbersbers
Copy link
Author

ComplexHeatmap::Heatmap(matrix(c(0, 0, 1, 1), nrow=2), show_row_dend = FALSE)

does not have this problem. So it's about the row dendograms. I could imagine that the problem is here:

if(is.null(max_height)) {
max_height = dend_heights(dend)
}
if(side %in% c("left", "right")) {
xscale = c(0, max_height)

Assume that dend == NULL. Then, dend_heights == 0 and, since max_height == NULL, max_height == 0 and xscale == c(0, 0). Boom.

If that is true, you may want to make sure xscale is not all zero.

Another fix may be around the caller to make sure row_dend_max_height != 0:

draw_dend(object, k = i, which = "row", y = slice_y[i], height = slice_height[i], just = "top",
max_height = row_dend_max_height)

Either way, good luck!

@jokergoo
Copy link
Owner

jokergoo commented Sep 8, 2021

Yes, thanks! That was because the height of the dendrogram is zero. Now I reset max_height to 1 if the dendrogram has zero height.

It is fixed and I have updated it on GitHub.

@bersbersbers
Copy link
Author

bersbersbers commented Sep 8, 2021

Thanks, 3e2efb6 does indeed fix the issue.

May I ask when you expect this fix to be available via Bioconductor? Also, my understanding is that R4.0 users will not be able to install anything newer than Bioconductor 3.12 anyway (is that right?), so I have a hard time figuring out what to put in a DESCRIPTION file (I would prefer not requiring R>=4.1, or making package users install from GitHub master). Maybe it might help if you tagged your releases on GitHub with the package version, is that possible?

Update: I just tried BiocManager::install(version="3.14"), which installed ComplexHeatmap_2.9.3, and that still has that issue. But apparently the builder only works once per day, even if you had pushed to git.bioconductor.org already.

Sorry for all these questions, but this is the first time I interact with a Bioconductor package. I think what I wonder most if this can be backported to Bioconductor 3.13 (the issue does not see to be present in 3.12).

@jokergoo
Copy link
Owner

jokergoo commented Sep 8, 2021

That is a little bit complicated for the management of Bioc packages. Current Bioc version is 3.13 and it corresponds to R 4.1.0. It only means the packages under bioc 3.13 are ensured to be stable with R 4.1.0. There might be problems if you use R packages earlier than 3.13 or use R < 4.1.0. The first scenario is more likely to happen because bioc packages (especially core packages) change their interface when evolving (e.g. GenomicRanges has changed a little bit internally), and the second scenario is less to happen only for some visualization packages which reply on grid may have problem with R-3.*.

However, use of R >=4.1 is not enforced. E.g. for ComplexHeatmap R>=4.0 is enough and I think generally putting R>=4.0 is OK unless you are submitting new packages to bioc.

For BiocManager::install(version="3.14"), because I haven't updated to bioc devel branch yet. I will do it later today. Then the version of ComplexHeatmap should be 2.9.4.

@bersbersbers
Copy link
Author

For BiocManager::install(version="3.14"), because I haven't updated to bioc devel branch yet. I will do it later today. Then the version of ComplexHeatmap should be 2.9.4.

Alright, good to know. Thanks!

However, use of R >=4.1 is not enforced. E.g. for ComplexHeatmap R>=4.0 is enough and I think generally putting R>=4.0 is OK unless you are submitting new packages to bioc.

Yes, R>=4.0 is enough - but BiocManager will not install 3.14 on R 4.0 (I guess - it does not even install 3.13).

I'll experiment some more once the package is in 3.14, thanks again!

@bersbersbers
Copy link
Author

bersbersbers commented Sep 18, 2021

Related:

Wednesday October 27th Release Bioconductor 3.14

https://support.bioconductor.org/p/9139541/

@bersbersbers
Copy link
Author

This is fixed in the just-released BioConductor 3.14 - thanks!

@bersbersbers
Copy link
Author

Here's another instance of the basically the same bug, using ComplexHeatmap 2.10 :(

ComplexHeatmap::Heatmap(
  matrix(c(1, 2), nrow = 2),
  right_annotation = ComplexHeatmap::rowAnnotation(
    anno = ComplexHeatmap::anno_barplot(c(0, 0))
  )
)

@jokergoo
Copy link
Owner

Thanks @bersbersbers ! There should be more places with this type of error. :)

@jokergoo jokergoo reopened this Nov 17, 2021
@bersbersbers
Copy link
Author

I guess, yes :) Fyi, this one is easier to workaround for the user by passing some ylim. In my case of purely non-negative values in anno_barplot, I can pass ylim = pmax(range(x), c(-Inf, .Machine$double.eps)) to avoid c(0, 0).

@jokergoo
Copy link
Owner

fixed :)

@bersbersbers
Copy link
Author

bersbersbers commented Nov 27, 2021

Thanks! I have twos concern about 826b321, though.

  1. Users who regularly plot values much small than 1 (let's say, standard deviations in [0, 1e-9]) will, if their standard deviations all are 0 for some reason, see the y scale switch to [0, 1] - meaning, the input values become smaller, but the y scale becomes bigger. This is somewhat counterintuitive. Also, this leads to as loss of information in the plot, as you have no way of seeing - on a [0, 1] scale - if the values are all 0 or simply small.

  2. Also, due to floating-point math, adding 1 can be a no-operation, see this example which returns 0:

    (1e16+1)-1e16

    I haven't tested it, but due to the above, I would expect the following code to still fail:

    ComplexHeatmap::Heatmap(
      matrix(c(1, 2), nrow = 2),
      right_annotation = ComplexHeatmap::rowAnnotation(
        anno = ComplexHeatmap::anno_barplot(c(1e16, 1e16))
      )
    )

What I would propose instead of adding 1 is to add a as small as possible epsilon (small due to issue 1, but big enough to prevent issue 2). In MATLAB, you can use x + eps(x) to get the next bigger floating point number:
https://www.mathworks.com/help/matlab/ref/eps.html

The pracma package has something similar:
https://www.rdocumentation.org/packages/pracma/versions/1.9.9/topics/eps

> x <- 0; x + pracma::eps(x) - x
[1] 2.225074e-308
> x <- 1e16; x + pracma::eps(x) - x
[1] 2
> x <- 1e100; x + pracma::eps(x) - x
[1] 1.942669e+84

This way, you can easily ensure that ylim consists of two numbers which are close enough such that the ylims print as same number while being numerically different.

@jokergoo
Copy link
Owner

Good point! I will update it according to your suggestion.

@jokergoo jokergoo reopened this Nov 29, 2021
@bersbersbers
Copy link
Author

(And maybe it would not hurt to do something like x + 1e3 * pracma::eps(x) instead of x + pracma::eps(x) to counteract any numerical issues later on.)

@bersbersbers
Copy link
Author

This thing is back again :) empty heatmaps work, but fail with an empty annotation:

ComplexHeatmap::Heatmap(matrix(nrow = 0, ncol = 5)) # works
ComplexHeatmap::Heatmap(matrix(nrow = 0, ncol = 5), # fails
  right_annotation = ComplexHeatmap::rowAnnotation(
    y = matrix(nrow = 0, ncol = 1), col = list(y = color)
  )
)

Error in valid.viewport(x, y, width, height, just, gp, clip, mask, xscale, :
invalid 'yscale' in viewport

@jokergoo
Copy link
Owner

Zero length annotation affects many places...

Is it important for you to have a real zero-length annotation?

@bersbersbers
Copy link
Author

Is it important for you to have a real zero-length annotation?

Important? No, not really - the app I am working on let's you filter your data, and some filters just return empty data. But I can easily check for empty data myself.

It would make cleaner code if I did not have to do that, though. Also, future users might save some time tracking down this same bug again. (What made me wonder was that empty heatmaps are working fine, so I thought it had to be something else this time.)

@jokergoo
Copy link
Owner

Because I considered empty heatmaps only :)

OK, then I will work on empty annotations. Current implementation always assumes there are data points in annotations.

@PeterK678
Copy link

Hi @jokergoo

I am having a similar error when Im trying to label specific genes in the heatmap. Could this be the cause for the issue?

Error in valid.viewport(x, y, width, height, just, gp, clip, mask, xscale, :
'x', 'y', 'width', and 'height' must all be units of length 1

vst_mat_z      #z-score matrix with all DEGs

Genes_Label = c("ANGPT1", "B3GNT8", "MGAT4B", "EDNRB", "NDNF", "ADAMTS15", "ST6GALNAC5", "BMP4", "CDH8", "COL21A1", "IL34", "ICAM1", "NOTCH4", "OMD", "SEMA3D", "VCAM1")

mark_at = which(rownames(vst_mat_z) %in% Genes_Label)

#Double-checking that rownames indices do correspond to original Genes_Label vector
Genes_DoubleCheck <- rownames(vst_mat_z)[mark_at]
setequal(Genes_DoubleCheck, Genes_Label) #TRUE if the two vectors contain the same elements regardless of their order.

ha = rowAnnotation(foo = anno_mark(at = mark_at, labels = Genes_DoubleCheck,
                  #link_width = unit(5, "mm"),
                  #padding = 0.5,
                  labels_gp = gpar(fontsize = 10
                     #, fontface = "bold"  
                                   )))

ht_selected_label <- Heatmap(vst_mat_z,
        col = col_fun,
        right_annotation = ha,
        rect_gp = gpar(col = "grey30", lwd = 0.15),
        show_column_names = F,
        show_row_names = TRUE,
        row_names_gp = gpar(fontsize = 5),
        clustering_distance_columns = "pearson",
        clustering_distance_rows = "pearson",
        column_names_gp = gpar(fontsize = 5),
        row_dend_reorder = FALSE, column_dend_reorder = TRUE,
        show_row_dend = F,
        row_dend_width = unit(10, "mm"),
        show_column_dend = F,
        width = unit(3, "in"), 
        height = unit(3.5, "in"), 
        heatmap_legend_param = lgd_list,
        name = "Z-score")

ht_selected_label = draw(ht_selected_label, heatmap_legend_side = "bottom")

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants