dphizler@gmail.com wrote:[color=blue]
> This is the basic concept of my code:
>
> #include <iostream.h>
> #include <math.h>
> #include <stdlib.h>[/color]
The first header is deprecated. User <iostream> (and "using std
namespace;"?). The latter two should usually be replaced with <cmath>
and <cstdlib>.
[color=blue]
> int main() {
> double Gtotal, L, size, distance, theRandom;
> double Gi[ 50 ][ 50 ][ 50 ];
> int xi, xf, yi, yf, zi, zf;[/color]
Don't declare variables until you can initialize them, and then make
those that don't need to change const and create the variables in as
small of a scope as possible. You might also consider using a container
instead of an array (see
http://www.parashift.com/c++-faq-lit....html#faq-34.1 and
http://www.parashift.com/c++-faq-lit...html#faq-13.10
and
http://www.parashift.com/c++-faq-lit...tml#faq-13.11).
[color=blue]
>
> Gtotal = 0; size = 50; L = 2;[/color]
Avoid putting more than one executable statement per line, and
initialize variables when you declare them.
[color=blue]
>
> // here every slot in the matrix has the same
> coordinates, to keep things simple
> // this code wasn't used to test this particular
> situation
> xi=2; xf=1; yi=2; yf=1; zi=2; zf=1;
>
> for(int x = 0; x < size; x++) {
> for(int y = 0; y < size; y++) {
> for(int z = 0; z < size; z++) {
> theRandom = (1 + rand()% 6 );
> if(theRandom <= 2) Gi[ x ][ y ][ z ] = 0;
> else Gi[ x ][ y ][ z ] = x - y + z;
> }}}
>
> for(int l = 0; l < size; l++) {
> for(int m = 0; m < size; m++) {
> for(int n = 0; n < size; n++) {
> for(int i = 0; i < size; i++) {
> for(int j = 0; j < size; j++) {
> for(int k = 0; k < size; k++) {
> if(Gi[ i ][ j ][ k ] != 0 || (i != l && j != m && k != n)) {
> distance = sqrt((xi - xf)^2 + (yi - yf)^2 + (zi - zf)^2);[/color]
First, I don't think you meant to use the xor operator (^) here. You
probably meant to use std::pow(). Second, since xi, xf, yi, yf, zi, and
zf don't change, calculate this outside the loop and make it a const.
[color=blue]
> Gtotal = Gtotal + (((1/distance) * exp(-distance/L)) * Gi[ i ][ j ][
> k ]);[/color]
This is more concise and efficient:
Gtotal += distanceConst * Gi[i][j][k];
where the constant distanceConst is computed outside the loop.
[color=blue]
> }
> }}}
>
> // normally I would have another array
> here instead of the same one
> // I just wanted to keep things simple
> Gi[ l ][ m ][ n ] = Gi[ l ][ m ][ n ] + Gtotal;
> cout << Gi[ l ][ m ][ n ] << endl;[/color]
Don't use std::endl, which inserts a cr-lf and then flushes the buffer.
Instead, use an ordinary '\n'.
[color=blue]
> }}}
>
> return 0;
> }
>
> Basic idea, I have a 3d array 50x50x50 that has some data in it.
> I'm trying to make a new 3d array based on the first one where each
> slot in the new array is generated from all 50x50x50 slots from the
> first 3d array.
>
> Now I know that
> (1/distance) * exp(-distance/L)
> will be redundant because the distance between two points in the array
> will be redundant.
>
> Try to imagine the 3d arrays being data in a cartesian graph where each
> slot has cartesian coordinates.
>
> Hopefully this is clear as to what I'm trying to do. I want to improve
> the the runtime.[/color]
Cheers! --M
