mbojan / rgraph6

Encoding graphs in graph6, sparse6, and digraph6 formats
https://mbojan.github.io/rgraph6/
GNU General Public License v3.0
12 stars 3 forks source link

Some Matrix coersion methods deprecate #33

Closed mbojan closed 9 months ago

mbojan commented 2 years ago

Martin Maechler writes:

Dear R Maintainer maintainer,

You are receiving this message because at least one CRAN or Bioconductor package that you maintain requires revisions due to deprecations in the forthcoming Matrix version 1.4-2, which you can install with

install.packages("Matrix", repos = "http://R-Forge.R-project.org")

(The list of 259 affected packages with 209 unique maintainers is at the end)

Matrix 1.4-2 will formally deprecate 187 coercion methods. More precisely, coercions of the form

as(object, Class)

where

  • 'object' inherits from the virtual class Matrix, is a traditional matrix, or is a logical or numeric vector

  • 'Class' specifies a non-virtual subclass of Matrix, such as dgCMatrix, but really any subclass matching the pattern

    ^dlnMatrix$

will continue to work as before but signal a deprecation message or warning (message in the widely used dg.Matrix and d.CMatrix cases).

By default, the message or warning will be signaled with the first deprecated method call and suppressed after that. Signaling can be controlled via option Matrix.warnDeprecatedCoerce:

<=0 = be completely silent [[ at your own risk ! ]] 1 = warn each time

=2 = error each time [[ for debugging ]] NA = message or warn once then be silent [[ the default ]]

Deprecated coercions in your package sources (including examples, tests, and vignettes) should be revised to go via virtual classes only, as has been advocated for quite some time in

vignette("Design-issues", package = "Matrix")

For example, rather than

as(<matrix>, "dgCMatrix")

we recommend (the full, a "permutation", or a simplification given the context of)

as(as(as(<matrix>, "dMatrix"), "generalMatrix"), "CsparseMatrix")

To simplify the revision process, the development version of Matrix provides Matrix:::.as.via.virtual(), taking a pair of class names and returning as a call the correct nesting of coercions:

Matrix:::.as.via.virtual("matrix", "dgCMatrix") as(as(as(from, "dMatrix"), "generalMatrix"), "CsparseMatrix") Matrix:::.as.via.virtual("matrix", "lsTMatrix") as(as(as(from, "lMatrix"), "symmetricMatrix"), "TsparseMatrix") Matrix:::.as.via.virtual("dgCMatrix", "dtrMatrix") as(as(from, "triangularMatrix"), "unpackedMatrix")

We suggest checking package tarballs built with

options(Matrix.warnDeprecatedCoerce = n) # where n >= 1

in the .onLoad() hook (see ?.onLoad), so that all deprecated coercions are exposed in the check output. (If you find that a warning or error has been signaled from 'Matrix' itself, then we'd welcome a report containing a minimal reproducible example, so that we may revise our own code.)

If you are unable to make the requested changes before Sep 8 (~4 weeks from now), or if you need help or clarification while fixing your code, please let us know by replying and copying

Matrix-authors@r-project.org

Thank you very much!

mbojan commented 2 years ago

With a follow-up:

Dear (Matrix-depending) package maintainers,

There have been several good questions as result of our e-mail on Friday. I will answer one of them here (inline) and add other remarks that have resulted from more such good questions.

Prof Dr Stephanie Evert on Sun, 14 Aug 2022 18:25:49 +0200 writes:

Dear Martin and Matrix authors, I have four questions about this change that I suspect may also be relevant to many other affected package maintainers.

My use of as(x, "dgCMatrix") in packages 'wordspace' and 'sparsesvd' ist that they include C code which operates only on the dgCMatrix format (and it would be extremely tedious and error-prone to duplicate this code for many other matrix formats). Inputs in other formats are thus converted to a dgCMatrix before processing.

My questions are thus:

1) Can I be sure that the result of the replacement coercion sequence you very conveniently suggested in your e-mail – i.e. as(as(as(, "dMatrix"), "generalMatrix"), "CsparseMatrix") – will always be a dgCMatrix?

Yes.

Or does the switch to coercions via virtual classes indicate that the class system will be changed in the near future?

No, not really. It's rather a sign that we, notably our new Matrix co-author Mikael Jagan, have been active in "cleaning up" things, and notably finally do some of the long-standing "TODO"s ..

One important remark however: In the mean time we have decided to keep

as(, "dgCMatrix")

non-deprecated as it is used in so many places. This is the only example of non-deprecation and is really only for traditional R matrices (class(.) |--> "matrix"), and not Matrix-package matrices.

Note that we have updated our development version of 'Matrix' on R-forge quite a bit since Friday: You may want to get (again) the latest development version by

install.packages("Matrix",
     repos = "http://R-Forge.R-project.org", type="source")

There were also quite a few changes to get rid of many of our own deprecation-causing cases during the weekend!

> 2) How memory-efficient is the three-step coercion
> sequence?  I have always used as(x, "dgCMatrix") to
> hopefully ensure that the conversion happens in a single
> step and no unnecessary copies are created in memory.  Or
> would the direct conversion have gone through intermediate
> steps, too, in this case?

Good question. Note that "this case" actually comprises many possible cases, quite a bit depending on class(x).

Also, when we wrote in Friday's e-mail

For example, rather than

as(, "dgCMatrix")

we recommend (the full, a "permutation", or a simplification given the context of)

as(as(as(, "dMatrix"), "generalMatrix"), "CsparseMatrix")

my parenthesized hint `` a "permutation" or a simplification given the context '' should have been made more clear and stronger.

Really, I'd rather quite strongly recommend to only replace as(, "dgCMatrix") by as(, "CsparseMatrix")

in most situations, where you already know from context that your (traditional R) matrix is numeric (and not 'logical'). In your case where you want to pass the result to C code which only works for "dgCMatrix" and nothing else, you may need to at least use the 2-step

as(as(, "CsparseMatrix"), "generalMatrix")

such that you don't get "dsCMatrix" (symmetricMatrix) or "dtCMatrix" (triangularMatrix)

but in most cases - where no restrictive C code is involved - your code, or rather the Matrix-package code you and your users would automatically use on the resulting matrix should work automatically (and more efficiently in some cases !!) also for the triangular or symmetric classed matrices that result from the simple as(, "CsparseMatrix")

which would be my own preference.

> 3) Using Matrix:::.as.via.virtual() to generate the sequence of coercions at runtime would be a convenient and reassuring solution for me, as it avoids unnecessary as() calls.  Is this just a temporary measure to ease the transition, or can package authors rely on it long-term?

This is just temporary (and hence not exported), not the least because the 3-step version is rarely what should really be used.

> 4) When can updated versions of my packages be submitted to CRAN? The strategy in 3) above wouldn't work at the moment because it requires version 1.4-2 of Matrix (for Matrix:::.as.via.virtual).  Would it work if I copy the logic of .as.via.virtual into my own packages in order to select a suitable conversion?

Indeed, this question of back-compatibility with earlier versions of Matrix is a also a very good one, I should have considered already in Friday's e-mail.

As a matter of fact, Jan Wijffels (also among the maintainers addressed) already found and asked me about on Friday that the example we gave for traditional "matrix"-class,

as(as(as(, "dMatrix"), "generalMatrix"), "CsparseMatrix")

indeed even fails directly in earlier versions of Matrix. So if you'd use it in your package source code, your package would need a Depends: Matrix( >= 1.4-2)

in the package's DESCRIPTION file. While this may be a safe solution for you as package maintainer, it is not so convenient even for you, notably because you'd have to wait for the CRAN release and of Matrix and only then can submit it to CRAN. Also, for some users, as Matrix is a recommended package and hence is (almost everywhere) installed together with the base R packages, in some environments it will lead to Matrix being installed in the original version that came with R and additionally in a newer version (>= 1.4-2 eventually) in the user's .LibPaths(). Typically all is fine with such a setup, but "you never know".

There's another back-incompatibility -- only concerning dense Matrix-pkg matrices : In 1.4-2 the "denseMatrix" class is conceptually "first" partitioned into "packedMatrix" and "unpackedMatrix" whereas these did not exist previously.

Now, when the result of Matrix:::.as.via.virtual(, "dgeMatrix") recommends to use as(, "packedMatrix") or as(, "unpackedMatrix") these would not be back-portable to Matrix < 1.4-2 either. Here you'd use -- also more readibly -- pack() and unpack(*) instead.

--

On another note, when in Friday's e-mail I recommended the lecture of

vignette("Design-issues", package = "Matrix")

I should have mentioned that the beginning of that vignette started incompletely, and it was mainly it's section 2 you should have used. In the mean time I've slightly updated that vignette and notably added the following link to my (DSC 2007) talk about the Matrix package class hierarchies (the talk was no longer online originally):

https://stat.ethz.ch/~maechler/R/DSC-2007_MatrixClassHierarchies.pdf

... Ok, this ended up becoming quite another non-short e-mail. I hope it will help you all in using the Matrix package successfully and with joy.

mbojan commented 2 years ago

Dear maintainers of CRAN or Bioconductor packages which depend (via 'Depends:' or 'Imports:') on our package 'Matrix'.

(CC CRAN team and a member of the Bioconductor Core team)

We plan to release a new version of Matrix , Matrix_1.5-0 -- currently already available from R-forge by running

 > install.packages("Matrix", repos="http://r-forge.r-project.org/")

and since yesterday installed on "Winbuilder R-devel", https://win-builder.r-project.org/upload.aspx If you upload your package there, note 'R-devel' (!), it will be checked against Matrix 1.5-0.

Some of you had been alerted before about the fact that Matrix 1.5-0 has deprecated many coercions, i.e., R function calls of the form

           as(<some>, "...Matrix")

Here, we repeat what some of you already got

More precisely, coercions of the form

   > as(object, Class)

where

      ^[dln]([gts][CRT]|di|ge|tr|sy|tp|sp)Matrix$

will continue to work as before but signal a deprecation message or warning (message in the widely used dg.Matrix and d.CMatrix cases). There's one exception we will continue allowing just because it has been in use in so many places:

    as(<matrix>, "dgCMatrix")   will not give a warning or message

By default, the message or warning will be signaled with the first deprecated method call and suppressed after that. Signaling can be controlled via

   option(Matrix.warnDeprecatedCoerce = <n>)

where n can take values (and has meaning)

<=0 : be completely silent  [[ at your own risk ! ]]
  1 : warn each time
>=2 : error each time  [[ for debugging ]]

NA : message or warn once then be silent  [[ the default ]]

Deprecated coercions in your package sources (including examples, tests, and vignettes) should be revised to go via virtual classes as has been advocated for quite some time in

> vignette("Design-issues", package = "Matrix")

For example, rather than

> as(<Matrix>, "dgCMatrix")

we recommend (the full, a "permutation", or a simplification given the context of)

> as(as(as(<Matrix>, "CsparseMatrix"), "generalMatrix"), "dMatrix")

To simplify the revision process, the development version of Matrix provides Matrix:::.as.via.virtual(), taking a pair of class names and returning as a call the correct nesting of coercions:

> Matrix:::.as.via.virtual("matrix", "lsTMatrix")
as(as(as(from, "lMatrix"), "symmetricMatrix"), "TsparseMatrix")
> Matrix:::.as.via.virtual("dgCMatrix", "dtrMatrix")
as(as(from, "triangularMatrix"), "unpackedMatrix")

Note that in practice we'd recommend to only use something like

for coercion from dense to a sparse matrix

    as( *, "CsparseMatrix")
or  as( *, "TsparseMatrix")
or  as( *,  "sparseMatrix") # in case it matters even less.

and for coercion to a dense matrix, maybe

    as( *, "denseMatrix")

or in the case you want a packed matrix, to use

    pack( as( *, "denseMatrix") )

and to let the Matrix-package's R-code (using the form of your matrix) decide what exact class the result should take.

We suggest checking package tarballs built with

> options(Matrix.warnDeprecatedCoerce = n) # where n >= 1

in the .onLoad() hook (see ?.onLoad), so that all deprecated coercions are exposed in the check output. (If you find that a warning or error has been signaled from 'Matrix' itself, then we'd welcome a report containing a minimal reproducible example, so that we may revise our own code.)

If you are unable to make the requested changes before Sep 8 (~4 weeks from now), or if you need help or clarification while fixing your code, please let us know by replying and copying


We have checked your packages with the new Matrix 1.5-0 installed AND with a severe (and non-default) setting of environment variable (to the value of 2)

   R_MATRIX_WARN_DEPRECATED_COERCE=2

which turns all the deprecation warnings mentioned above into ('fatal') errors.

For all your packages, when 'R CMD check'ing them, we found problems with the new version of Matrix (and the above severe option to turn the warnings into fatal errors).

You can access these problems in

https://stat.ethz.ch/~maechler/R/PkgErr2/<pkg>.Rout

e.g., for my own MatrixModels package in

https://stat.ethz.ch/~maechler/R/pkgErr2/MatrixModels.Rout

Now, for the CRAN or Bioconductor repository checks, R_MATRIX_WARN_DEPRECATED_COERCE=2 will not be set, so you may not see anything on your package check package, nor when you use Win-builder or your own 'R CMD check --as-cran ' unless you add such an option setting to your tests/* R code files,

  options(Matrix.warnDeprecatedCoerce = 2) # being the strongest

or set the R_MATRIX_WARN_DEPRECATED_COERCE setting as mentioned above.

Please use the above URL to see the problem(s) with our package(s) and solve them within the next few weeks ideally. Note that 'testthat' users will not see the deprecation warnings, as there warnings are not shown, as far as we know, but just counted (by default). The level 2 setting will make the warnings into errors, so you will notice it, as we have in your respective https://stat.ethz.ch/~maechler/R/pkgErr2/.Rout .

Please ask here (Matrix-author@r-project.org) if you need advice or help.