[Bioc-devel] RMassBank build error
Laurent Gatto
lg390 at cam.ac.uk
Mon Oct 10 12:32:25 CEST 2016
On 10 October 2016 08:46, Stravs, Michael wrote:
> Hi Laurent,
>
>> I don't think that your initialize method will work as it is, because it passess
>> arguments to callNextMethod, which doesn't have them. But, to be honest, I
>> don't think that any intialize method will sort the current problem.
>
> The method did work with MSnbase until the recent constructors
> switch... The whole point of the method was to make a copy constructor
> possible by explicitely assigning the arguments ("Versioned" has a
> problem), otherwise it would do something strange.
>
> But yes, I don't think it will solve the current problem.
>
>>
>> I am not sure how to best handle to problem. To sort your problem, we (the
>> MSnbase maintainers) could revert to our old initialize,Spectrum method,
>> but some very weird bugs (in R? in some very low-level C code?) result in
>> random crashes. We are looking for something else.
>
> But that's weird as is, no? I thought your constructor did only
> relatively mundane stuff, so if this causes problems then it shouldn't
> be MSnbase's responsibility to fix them and they are alarming for
> every R programmer solution out there...
I don't think we are at that stage yet, and I wouldn't make any strong
claims about there being a bug in R. MSnbase and it's reliance on mzR
and proteowizard opens up many possibilities for bugs and weirdness.
>> Anyway, if we don't manage to find an acceptable solution for MSnbase, I will
>> submit an RMassBank patch that will sort the error (and other things resulting
>> from MSnbase improvements) out. What would be the best way to submit a
>> patch - email, a pull request to a github repo?
>
> A pull request to github.com/MassBank/RMassBank would be best I
> think. Even though the version on there is possibly newer than the one
> on Bioc.
>
> I am currently really busy with urgent stuff and therefore RMassBank
> development is on the far-backburner but there's a ton of things I'd
> like to implement. Is there a summary of MSnbase improvements that I
> could look at, since there were apparently quite many? Maybe there's
> other things we could benefit from.
Latest news is that we should be able to fix this on our side. If not, I
have an RMassBank patch ready to be sent as a github PR.
(I would suggest you add a URL field in your DESCRIPTION file, to
document that github repo instead of the Bioconductor-mirror one.)
> Also, will there be a problem with regards to serialization? We relied
> quite heavily on load()ing and save()ing large collections of
> RMassBank objects including MSnbase spectra; is there much I will need
> to consider in the upgrade path when loading?
The NEWS file (news(package = "MSnbase")) and the benchmarking vignette
[1] are good places to see recent changes and read about the the new
version. In a nutshell, there's now an on-disk back-end, that does not
read the spectrum data into memory when creating the MSnExp raw data
object, but only on demand, when actually needed. The benefits are
described in the vignette.
This has a big effect on serialisation: as we don't have support to
write mzML data, serialisation is not possible with on-disk data. That's
however not too much of a problem because (1) the in-memory back-end is
still available (and will remain available, albeit probably won't
benefit from recent developments as much as the new one) and (2) it is
possible to go from on-disk to in-memory before saving.
I will try to document differences more systematically in this issue [2]
in a first instance and then in the package documentation.
Best wishes,
Laurent
[1] http://bioconductor.org/packages/devel/bioc/vignettes/MSnbase/inst/doc/benchmarking.html
[2] https://github.com/lgatto/MSnbase/issues/165
> Thanks and best wishes,
> -Michele
>
>
>>
>
>
>> -----Original Message-----
>> From: Laurent Gatto [mailto:lg390 at cam.ac.uk]
>> Sent: Sonntag, 9. Oktober 2016 22:03
>> To: Stravs, Michael
>> Cc: Rainer Johannes; Schymanski, Emma; Martin Morgan; sneumann at ipb-
>> halle.de; erik.mueller at ufz.de; Martin Morgan
>> Subject: Re: [Bioc-devel] RMassBank build error
>>
>>
>> Dear Michael and RMassBank developers,
>>
>> On 9 October 2016 15:53, Stravs, Michael wrote:
>>
>> > Hi all,
>> >
>> > I now have all packages installed to test things and can reproduce the
>> > problem. I must confess that I don't have a deep understanding on how
>> > S4 initialization works, so solution attempts on my side are very
>> > trial-and-errory.
>> >
>> > I will have to look into the workings of these initialization methods
>> > anyway in the future. The RMassBank version currently on Bioc-devel
>> > uses the default constructor (i.e. doesn't actually define an "initialize"
>> > method).
>> >
>> > However, one of my development versions has an initialize method,
>> > which I introduced to work around a bug with Versioned initialize (see
>> > my question here [1], the reply [2], and my current code [3]). That
>> > one is
>> > *also* incompatible with the new MSnbase constructors because of some
>> > argument mapping issues (but there might also be a workaround for
>> > that.)
>> >
>> > [1] https://stat.ethz.ch/pipermail/bioc-devel/2016-January/008528.html
>> > [2] https://stat.ethz.ch/pipermail/bioc-devel/2016-January/008576.html
>> > [3]
>> >
>> https://github.com/MassBank/RMassBank/blob/s4power/R/SpectrumMeth
>> ods.R
>> > #L124
>>
>> I don't think that your initialize method will work as it is, because it passess
>> arguments to callNextMethod, which doesn't have them. But, to be honest, I
>> don't think that any intialize method will sort the current problem.
>>
>> I am not sure how to best handle to problem. To sort your problem, we (the
>> MSnbase maintainers) could revert to our old initialize,Spectrum method,
>> but some very weird bugs (in R? in some very low-level C code?) result in
>> random crashes. We are looking for something else.
>>
>> Anyway, if we don't manage to find an acceptable solution for MSnbase, I will
>> submit an RMassBank patch that will sort the error (and other things resulting
>> from MSnbase improvements) out. What would be the best way to submit a
>> patch - email, a pull request to a github repo?
>>
>> Best wishes,
>>
>> Laurent
>>
>> > On 07.10.2016 22:34, Laurent Gatto wrote:
>> >> On 7 October 2016 21:30, Rainer Johannes wrote:
>> >>
>> >>> an update:
>> >>>
>> >>> I've switched back to the "default" initialize methods for Spectrum1
>> >>> and Spectrum2 objects in MSnbase (not implemented in C) and with
>> >>> these installing RMassBank seems to work.
>> >>> I have however now to run some intense torture tests on MSnbase to
>> >>> ensure that we don't run again into the random memory problems that
>> >>> we had in MSnbase (issue
>> >>> https://github.com/lgatto/MSnbase/issues/138)
>> >> I have done the same thing, which also needed addressing some unit
>> >> tests. I will push these updates to master for after a final package
>> >> check, but wait for the results of your intense torture tests before
>> >> committing to svn.
>> >>
>> >> Laurent
>> >>
>> >>> jo
>> >>>
>> >>>
>> >>>> On 7 Oct 2016, at 20:14, Rainer Johannes
>> <Johannes.Rainer at eurac.edu> wrote:
>> >>>>
>> >>>> we could try to switch back from the C-constructors to the "old"
>> >>>> ones, I'll check later
>> >>>>
>> >>>>> On 7 Oct 2016, at 20:03, Schymanski, Emma
>> <Emma.Schymanski at eawag.ch> wrote:
>> >>>>>
>> >>>>> Hi Laurent,
>> >>>>>
>> >>>>> I don't have an answer for you right now - but in CC are also the other
>> 3 involved in trying to fix this on our side...
>> >>>>> Just to make sure that all have the respective email addresses to try
>> speed up the debugging...
>> >>>>>
>> >>>>> Thanks!
>> >>>>> Emma
>> >>>>> ________________________________________
>> >>>>> From: Laurent Gatto [lg390 at cam.ac.uk]
>> >>>>> Sent: Friday, 7 October 2016 7:52 PM
>> >>>>> To: Schymanski, Emma; Martin Morgan
>> >>>>> Cc: bioc-devel at r-project.org; Rainer Johannes
>> >>>>> Subject: Re: [Bioc-devel] RMassBank build error
>> >>>>>
>> >>>>> Dear Emma,
>> >>>>>
>> >>>>> The error is very strange indeed, and I hope Martin can help us out
>> here.
>> >>>>>
>> >>>>> With the latest RMassBank and MSnbase, I get
>> >>>>>
>> >>>>>> new("RmbSpectrum2")
>> >>>>> Error in initialize(value, ...) :
>> >>>>> 'initialize' method returned an object of class “Spectrum2”
>> >>>>> instead of the required class “RmbSpectrum2”
>> >>>>>
>> >>>>> which reproduces the error.
>> >>>>>
>> >>>>> The RmbSpectrum2 class is defined in a standard way
>> >>>>>
>> >>>>> .RmbSpectrum2 <- setClass("RmbSpectrum2",
>> >>>>> representation = representation(
>> >>>>> satellite="logical",
>> >>>>> low="logical",
>> >>>>> rawOK ="logical",
>> >>>>> good = "logical",
>> >>>>> mzCalc = "numeric",
>> >>>>> formula = "character",
>> >>>>> dbe = "numeric",
>> >>>>> formulaCount = "integer",
>> >>>>> dppm = "numeric",
>> >>>>> dppmBest = "numeric",
>> >>>>> ok = "logical",
>> >>>>> info = "list"
>> >>>>> ),
>> >>>>> contains=c("Spectrum2"),
>> >>>>> prototype = prototype(
>> >>>>> satellite = logical(),
>> >>>>> low = logical(),
>> >>>>> rawOK = logical(),
>> >>>>> good = logical(),
>> >>>>> mzCalc = numeric(),
>> >>>>> formula = character(),
>> >>>>> dbe = numeric(),
>> >>>>> formulaCount = integer(),
>> >>>>> dppm = numeric(),
>> >>>>> dppmBest = numeric(),
>> >>>>> ok = logical(),
>> >>>>> info = list(),
>> >>>>> new("Versioned",
>> versions=c(classVersion("Spectrum2"),
>> >>>>> RmbSpectrum2 = "0.1.0"))
>> >>>>> ))
>> >>>>>
>> >>>>>
>> >>>>> and
>> >>>>>
>> >>>>>> new("Spectrum2")
>> >>>>> Object of class "Spectrum2"
>> >>>>> Precursor: NA
>> >>>>> Retention time: :
>> >>>>> Charge: NA
>> >>>>> MSn level: 2
>> >>>>> Peaks count: 0
>> >>>>> Total ion count: 0
>> >>>>>
>> >>>>> works.
>> >>>>>
>> >>>>> The MSnbase maintainers have had a bit of a struggle with spurious
>> >>>>> and stange failures in the recent past (see [1]). This ans a whole
>> >>>>> new backend in the package have led to the following initialize
>> >>>>> method, that constructs the class directly in C
>> >>>>>
>> >>>>> setMethod("initialize",
>> >>>>> "Spectrum2",
>> >>>>> function(.Object, msLevel = 2L, peaksCount = length(mz),
>> >>>>> rt = numeric(), acquisitionNum = NA_integer_,
>> >>>>> scanIndex = integer(), tic = 0L, mz = numeric(),
>> >>>>> intensity = numeric(), fromFile = numeric(),
>> >>>>> centroided = NA, smoothed = NA,
>> >>>>> polarity = NA_integer_, merged = 1,
>> >>>>> precScanNum = NA_integer_, precursorMz = NA,
>> >>>>> precursorIntensity = NA, precursorCharge = NA_integer_,
>> >>>>> collisionEnergy = NA) {
>> >>>>> .Object <- Spectrum2_mz_sorted(msLevel, peaksCount, rt,
>> >>>>> acquisitionNum, scanIndex,
>> >>>>> tic, mz, intensity, fromFile,
>> >>>>> centroided, smoothed, polarity,
>> >>>>> merged, precScanNum, precursorMz,
>> >>>>> precursorIntensity, precursorCharge,
>> >>>>> collisionEnergy)
>> >>>>> if (validObject(.Object))
>> >>>>> .Object
>> >>>>> })
>> >>>>>
>> >>>>>
>> >>>>> Why is calling new("RmbSpectrum2") direclty returning an Spectrum2
>> >>>>> object? Is this due to us not calling callNextMethod?
>> >>>>>
>> >>>>> Laurent
>> >>>>>
>> >>>>> [1] https://github.com/lgatto/MSnbase/issues/138
>> >>>>>
>> >>>>>
>> >>>>> On 7 October 2016 17:02, Schymanski, Emma wrote:
>> >>>>>
>> >>>>>> Hi BioC team,
>> >>>>>>
>> >>>>>> In all the mzR troubles, it slipped through that RMassBank had a build
>> error of it's own presumably caused by an update to MSnbase - for some
>> reason we never received the email with the build error reports?!
>> >>>>>> We have discovered the error now, very late, and are onto finding
>> out the cause to fix it but it is not straightforward. Now that it is the day of
>> the deadline to pass build without error, are we able to have a little leeway if
>> needed? It's taken us the whole day to get the right binaries to actually have
>> a chance to start fixing...
>> >>>>>> Can someone also check or explain why we no longer receive the
>> emails reporting errors to us?
>> >>>>>>
>> >>>>>> Thanks,
>> >>>>>> Emma (on behalf of the others)
>> >>>>>> _______________________________________________
>> >>>>>> Bioc-devel at r-project.org mailing list
>> >>>>>> https://stat.ethz.ch/mailman/listinfo/bioc-devel
>> >>>>>
>> >>>>> --
>> >>>>> Laurent Gatto | @lgatt0
>> >>>>> http://cpu.sysbiol.cam.ac.uk/
>> >>>>> http://lgatto.github.io/
>> >>
>>
>>
>> --
>> Laurent Gatto | @lgatt0
>> http://cpu.sysbiol.cam.ac.uk/
>> http://lgatto.github.io/
--
Laurent Gatto | @lgatt0
http://cpu.sysbiol.cam.ac.uk/
http://lgatto.github.io/
More information about the Bioc-devel
mailing list