[Bioc-devel] RMassBank build error

Stravs, Michael Michael.Stravs at eawag.ch
Mon Oct 10 09:46:16 CEST 2016


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...

> 
> 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.

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?

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/


More information about the Bioc-devel mailing list