[Rivet] problem with Analysis::divide?

[James Monk]

Hi Andy,

yes, sorry this slipped down the radar - in the end I realised that I needed to implement the analysis using the old aida datapointsets for the released version of Rivet anyway, but I know you'll want to get this sort of thing fixed.  I believe the problem is not that the *s that is being assigned to is an issue, but that a reference to a temporary is being returned, i.e. effectively the same as this:

	Scatter2D tmp = *h1 / *h2;  //copy result from YODA / operator
	*s = &tmp; // assign tmp to *s pointer that was passed in
	return;
	// tmp is now out of scope and is removed from the stack.  *s points to junk at this point

I *think* that the jet veto analysis (ATLAS_2011_S9126244) also suffers from this problem (that would make sense, since its final plots are also ratios).  At any rate, whenever I've run that analysis using YODA I get a load of broken plots.

If you still need it, then I can try and contrive a "minimal" example, but maybe you don't?

cheers,

James


On 4 Mar 2013, at 13:48, Andy Buckley wrote:

> Hi James,
> 
> Hendrik and I are having a bit of a Rivet-YODA hacking session now, and
> obviously this is a major thing to resolve.
> 
> I'm puzzled by your problem... the point of the *s = h1 / h2 as far as I
> recall is that it's the dereferenced scatter pointer *s that is being
> assigned to, so there should be a Scatter2D copy assignment operation
> here. So the temporary is copied before it's returned... or should be.
> 
> As Hendrik asked, do you have a minimal example that segfaults for you?
> 
> Andy
> 
> 
> On 14/12/12 13:05, James Monk wrote:
>> Anyone..?  In case I wasn't clear, this is causing a seg-fault at runtime
>> 
>> James
>> 
>> On 10 Dec 2012, at 19:30, James Monk wrote:
>> 
>>> Hi,
>>> 
>>> I think there's a problem with the histogram divide method on the YODA-fied version of Rivet.  It takes a pointer to a scatter plot for the result, thus:
>>> 
>>> 	  void Analysis::divide(const Histo1D & h1, const Histo1D & h2, Scatter2DPtr s) const
>>> 
>>> and then assigns 
>>> 
>>> 	*s = h1 / h2;
>>> 
>>> but that / operator returns a temporary that is not valid outside of the divide method.  I'm not sure what the intent is here - should it be 
>>> 
>>> 	s = new Scatter2DPtr(h1 / h2);
>>> 
>>> but in that case, why not do away with passing the pointer in and just return it? 
>>> 
>>> cheers,
>>> 
>>> James
>>> _______________________________________________
>>> Rivet mailing list
>>> Rivet at projects.hepforge.org
>>> http://www.hepforge.org/lists/listinfo/rivet
>> 
>> _______________________________________________
>> Rivet mailing list
>> Rivet at projects.hepforge.org
>> http://www.hepforge.org/lists/listinfo/rivet
>> 
> 
> 
> -- 
> Dr Andy Buckley, Royal Society University Research Fellow
> Particle Physics Expt Group, University of Edinburgh
> 
> The University of Edinburgh is a charitable body, registered in
> Scotland, with registration number SC005336.
>