Can you spot my mistake?

B

boyfarrell

Registered
Hi all,

Now this looks a bit messy! But actually it's quite simple. It basically multiplies vectors together. However, I have a vector at a point in 3D space. That's what a PointVector class is. The code is part of a finite element method of solving a problem in material science.

When I run this code I get a a good old 10 (SIGBUS). It has something to with the release of myPtVecTermOnePreFactor (which is an array of Vector* type objects) in the last for loop of the method. When I don't release it the code runs fine?

Any ideas? (Come on Viro it's your time to show off again! :D )

Code: 
-(PointVector*) calculateTermOneIntegrandMeshPointsWithPtVec: (PointVector*) myPtVecTermOneIntPartCosh
 And: (PointVector*) myPtVecSolution 
AndChempot: (ChemicalPotential*) myChempot;
{
	
	//Make term one prefactor vectors
	Vector *myPtVecTermOnePreFactor[ptsD];
	
	unsigned int q;
	for (q=0; q <= (ptsD-1); q++)
	{

		//fill with term one prefactor
		myPtVecTermOnePreFactor[q] = [[Vector alloc] init];
		myPtVecTermOnePreFactor[q] = [self termOneFactorAtDepth:q];
		
		//Calculate the Cosh term for different points
		[self termOneIntegrandPartCoshForFluxDirectionZAtDepth: q];
		
		//We now want add like frequency elements at different positions to reduce the ptsD number of
		//Vector* to one Vector then place this at the correct point of interest in the 'termOneIntegrandPartCosh' instance.
		[self termOneIntegrandPartCoshActionSummateLikeFrequencyElementsForFluxDirectionZ: myPtVecTermOneIntPartCosh];
	}
	
	//create an instance to hold the brightness values
	PointVector *myPtVecBrightnessDistribution = [[PointVector alloc] initAll];
	//fill with values
	[self termBrightnessWithChemicalPotentialDistribution: myChempot UsingPointVector: myPtVecBrightnessDistribution];
	
        //PointVector muliplication
	[myPtVecSolution equalsPointVector: myPtVecTermOneIntPartCosh TimesPointVector: myPtVecBrightnessDistribution];

	//times by the prefactor
	unsigned int j,i;
	for (q=0; q <= (ptsD-1); q++)
		for (j=0; j <= (ptsW-1); j++)
			for (i=0; i <= (ptsL-1); i++)
			{
				[myPtVecSolution timesVector: myPtVecTermOnePreFactor[q] ToPointI:i J:j Q:q];
				[myPtVecTermOnePreFactor[q] release];
			}
				
				
	
	
	[myPtVecBrightnessDistribution release];
					
	return myPtVecSolution;
	
}
 
At first glance, I think the problem lies with the line [myPtVecTermOnePreFactor[q] release];. You are releasing that same object many times through the 'j' and 'i' loops without changing the value of 'q'.
 
I think you might want to change that last loop to this:
Code: 
for (q=0; q <= (ptsD-1); q++)
{
	for (j=0; j <= (ptsW-1); j++)
	{
		for (i=0; i <= (ptsL-1); i++)
		{
			[myPtVecSolution timesVector: myPtVecTermOnePreFactor[q] ToPointI:i J:j Q:q];
		}
	}
	
	[myPtVecTermOnePreFactor[q] release];
}
 
Errr...

Actually, now I get 11 (SIGSEGV)! I must be trying to use that object after I release it? I've check all the methods that it uses, they all pass by value. So I can't see how I'll have any pointers hanging around?
 
Well, you can try changing the line
Code: 
myPtVecTermOnePreFactor[q] = [[Vector alloc] init];
to
Code: 
myPtVecTermOnePreFactor[q] = [[[Vector alloc] init] autorelease];
and then delete the line
Code: 
[myPtVecTermOnePreFactor[q] release];
 
Ok, here's a modified (untested!) version that uses NSArray, which is more Cocoa-ish-like. Also, termOneFactorAtDepth: might be causing issues. If this code still crashes, we need to see the code for that method too..
Code: 
-(PointVector*) calculateTermOneIntegrandMeshPointsWithPtVec: (PointVector*) myPtVecTermOneIntPartCosh
 And: (PointVector*) myPtVecSolution 
AndChempot: (ChemicalPotential*) myChempot;
{
	
	//Make term one prefactor vectors
	NSMutableArray *myPtVecTermOnePreFactor = [NSMutableArray arrayWithCapacity:ptsD];
	unsigned int q;

	for (q=0; q <= (ptsD-1); q++)
	{
		//fill with term one prefactor
		[myPtVecTermOnePreFactor addObject:[self termOneFactorAtDepth:[[[Vector alloc] init] autorelease]]];
		
		//Calculate the Cosh term for different points
		[self termOneIntegrandPartCoshForFluxDirectionZAtDepth: q];
		
		//We now want add like frequency elements at different positions to reduce the ptsD number of
		//Vector* to one Vector then place this at the correct point of interest in the 'termOneIntegrandPartCosh' instance.
		[self termOneIntegrandPartCoshActionSummateLikeFrequencyElementsForFluxDirectionZ: myPtVecTermOneIntPartCosh];
	}
	
	//create an instance to hold the brightness values
	PointVector *myPtVecBrightnessDistribution = [[[PointVector alloc] initAll] autorelease];
	//fill with values
	[self termBrightnessWithChemicalPotentialDistribution: myChempot UsingPointVector: myPtVecBrightnessDistribution];
	
        //PointVector muliplication
	[myPtVecSolution equalsPointVector: myPtVecTermOneIntPartCosh TimesPointVector: myPtVecBrightnessDistribution];

	//times by the prefactor
	unsigned int j,i;
	for (q=0; q <= (ptsD-1); q++)
		for (j=0; j <= (ptsW-1); j++)
			for (i=0; i <= (ptsL-1); i++)
				[myPtVecSolution timesVector: [myPtVecTermOnePreFactor objectAtIndex:q] ToPointI:i J:j Q:q];
					
	return myPtVecSolution;
}
Also,
Code: 
for (q=0; q < ptsD; q++)
is an easier way of what you have,
Code: 
for (q=0; q <= (ptsD-1); q++)
;)
 
The code is fine when you autorelease. However, when you loop through the object to release it it always give an run time error in either version, Cocoa or standard C array of objects.

Something must be fishy with the the -termOneFactorAtDepth: method as you suggest. Here is that method. termOneFactorAtDepth creates and returns/autoreleases a Vector class object. It fills the values of the Vector by messaging and looping through another object called -termOneFactorAtElement:AtDepth:

Code: 
-(Vector*) termOneFactorAtDepth: (unsigned int) q;
{
	Vector *myVector = [[Vector alloc] init];
	
	unsigned int f;
	for (f=0; f <= (ptsF-1) ; f++)
		[myVector setColumnAtIndex: f To: [self termOneFactorElement: f AtDepth: q] ];
	
	return [myVector autorelease];
}

-(double) termOneFactorElement: (unsigned int) f AtDepth: (unsigned int) q;
{
	double ang = pow((cos([self theta]/2)),2);
	
	return (
			/* numerator*/		(
								 [self omegaC]*[self nse:f]
								 *cosh( ([self nsa:f]*[self z:q]/ang) + ([self aL]/2) )
								 )
			
			/	
			
			/*denominator*/	(
							 2*M_PI*ang*sinh(([self nsa:f]*[self d]/ang) + [self aTB])
							 )
			);
}

I guess it has something to do with allocting and autoreleasing the Vector* in termOneFactorAtDepth:

Daniel
 
I have changed the -termOneFactorAtDepth: method to just create and the fill vector with values. I have then made the messaging method responsible for releasing these Vector objects. Seemed to solve the problem.

Dan.

Just for completeness here is the final code:
Code: 
-(Vector*) termOneFactorAtDepth: (unsigned int) q;
{
	Vector *myVector = [[Vector alloc] init];
	
	unsigned int f;
	for (f=0; f < ptsF; f++)
		[myVector setColumnAtIndex: f To: [self termOneFactorElement: f AtDepth: q] ];
	
	return myVector;// this used to be [myVector autorelease];

//BECAREFUL. Responsibility for releasing has been passed to the messaging method. 
}

Is this standard practice?
 
If you've already made them autorelease when you return from -termOneFactorAtDepth:, I'm fairly sure you do not need to loop through the elements of the array to manually release them. That would completely defeat the purpose of autorelease.
 
Yeah, that's what I've done. Just returned the object from -termOneFactorAtDepth: and then made the messaging method responsible for releasing it.
 
You must log in or register to reply here.
Back
Top