Setting up array

[lmd141]

Hi there, I have a large array of doubles that are inputted from a text file in the following way:

void MOF5_coords(int &atom_types, double *&sigma_point, double *&epsilon_point, double *&mass_point, int &atom_number, double *&x_point, double *&y_point, double *&z_point, int *&type_point)
{        
string dummystring;
            string s;

            ifstream Dfile;
            std::stringstream out;

            out << 2;
            s = out.str() + ".TXT";
            Dfile.open (s.c_str());
			
			if (Dfile.fail())
            {
                return;
            }
            for (int i=0; i<atom_number; i++)
			{
            Dfile >> type_point[i];
			Dfile >> x_point[i];
			Dfile >> y_point[i];
			Dfile >> z_point[i];
			}

			Dfile.close();
}

Now this works fine, I have x_point, y_point and z_point (a 3D structure). I want to then set up a coordinate system, something along the lines of:

	double **coords = new double*[n_atoms_arb];
	for (int i = 0; i < n_atoms_arb; i++)
{
	coords[i] = new double[N];
}
	coords[3] = {x_point, y_point, z_point};

    for (int i=0; i<atom_number; i++)
    {
		coords[i] = new double[3];
 		
		for (int j = 0; j < 3; j++)
			coords[i][j] = {x_point, y_point, z_point};
    }

But I understand there's a lot of faults in this code (and method!). Does anyone have a better suggestion of doing this for me?

Cheers

[Imadatobanisa]

Now you tell us about your problem that currently makes you sad.

1- Description?

2- Program faults? Be specific. What?

3- Any wrong final result, output?

Hope this helps.

[kg1992]

Hello.

Some minor concerns that you might don't really have to care, but let me just write down.

1. Organizing code with indentation is good idea.

2. Commenting is good idea, too.

3. Having Naming convention is also good idea.

4. What is the purpose of dummystring at line 3?

5. Make MOF5_coords function return something so programmer using your function can test if the function succeeded or not.

6. passing too much arguments to function is usually not a good idea because it is hard to remember.

7. Do you call delete at the end of the program?

8.

Dfile >> type_point[i]
Dfile >> x_point[i]
...

This is actually same as this

Dfile >> type_point[i] >> x_point[i] >> ...

9. If you leaned how to, it is good idea to make struct. Here is my suggestion for struct "Atom"

struct Atom
{
  double sigma;
  double epsilon;
  double mass;
  double x;
  double y;
  double z;
  int type;

};

Then, you can do this.

std::ifstream& operator>>( std::ifstream& in, Atom& atom )
{
  in >> atom.type >> atom.x >> atom.y >> atom.z;

  return in;
}

Dfile >> atom_array[i]; // Done 

If you didn't lean how to do this, you don't have to think about this now.

[lmd141]

Hi, realized I was probably a bit lean with the information here. This is part of a monte carlo scheme modelling the movement particles inside a simulation space. The purpose of the text file is so that the program reads thousands of particle coordinates quickly and easily.

Originally I set up a coordinate system as an array of an arbitrary length (it wasn't really important). Something along the lines of this:

	// Setting up coordinate system for simulation

	double **coords = new double*[n_atoms_arb];
	for (int i = 0; i < n_atoms_arb; i++)
{
	coords[i] = new double[N];
}

 // Randomly generate the coordinates of the atoms in the box
    for (int i = 0; i < n_atoms; i++)
    {
		coords[i] = new double[3];
 		for (int j = 0; j < 3; j++)
        coords[i][j] = RanGen.Random()*box_size[j];
    }

However, what I want to do now is - instead of setting up the coordinates as an arbitrarily long array and then generating random coordinates inside the space - I want to set them up to be the x_point, y_point and z_points (a very long array of points) mentioned in my first post.

There are no problem faults as I'm coming up with a scheme for doing this rather than correcting some code I've written. Basically what I am pondering is how do I keep the style similar to the method I used originally, while using the (x,y,z) coordinates inputted from the text file instead of an arbitrarily long array.

Thanks for the comments kg, some of the ones you mention are actually done in my program but I just didn't copy them over. The dummystring at line 3 is there because in some other text files I have used there are values that I do not want the compiler to use (such as text).

[kg1992]

Hello.

It is not allowed to do something like this:

coords[i] = {x_point[i], y_point[i], z_point[i]};

That list of numbers including curly braces on the right hand side is called "Array Initialization List", which is allowed only when you "initialize" the array.


From what I know, I can think of no better solution than assigning them one by one like this:

const int COORD_X_INDEX = 0;
const int COORD_Y_INDEX = 1;
const int COORD_Z_INDEX = 2;
for( int i = 0; i < n_atoms; i++ )
{
    coords[i][COORD_X_INDEX] = x_point[i];
    coords[i][COORD_Y_INDEX] = y_point[i];
    coords[i][COORD_Z_INDEX] = z_point[i];
}

Or If you want to save a little more line, notice that if you line up x_point, y_point, and z_point in row and read in vertical way, one column becomes each point.

/*x_point*/ { 1, 2, 3, 4, 5, ...}
/*y_point*/ { 2, 3, 4, 5, 6, ...}
/*z_point*/ { 3, 4, 5, 6, 7, ...}
-> {1, 2, 3}, {2, 3, 4}, {3, 4, 5}, ... if you read column by column.

Then this is possible:

// Create array of pointer to doubles.
double* xyz[] = { x_point, y_point, z_point };
// Array is treated as pointer here.
// array name in expression returns pointer to the first element of the array.

for( int i = 0; i < n_atoms; i++ )
{
  for( int j = 0; j < 3; j++ )
  {
    coords[i][j] = xyz[j][i];
  }
}

Sorry if I misunderstand what you want to do again. English is not my native language.

[Darkmaster]

imo the easiest way to do this is std::vector and the member function pushback().
also you dont need to know how many elements your text file contains.

[lmd141]

Hello again. Thanks for the help guys, kg your method was exactly what I was looking for! Cheers.

Just one quick question though. My code now looks like this (the methane/MOF5_coords function are the same as my original post at the top):

void methane_coords(int &atom_types, double *&sigma_point, double *&epsilon_point, double *&mass_point, int &atom_number, double *&x_point, double *&y_point, double *&z_point, int *&type_point)


void MOF5_coords(int &atom_types, double *&sigma_point, double *&epsilon_point, double *&mass_point, int &atom_number, double *&x_point, double *&y_point, double *&z_point, int *&type_point)

int main()
{

methane_coords(atom_types, sigma_point, epsilon_point, mass_point, atom_number, x_point, y_point, z_point, type_point);

double **coords_fluid = new double*[n_atoms_methane];
for (int i = 0; i < n_atoms_methane; i++)
{
	coords_fluid[i] = new double[N];
}

const int COORD_X_INDEX_FLUID = 0;
const int COORD_Y_INDEX_FLUID = 1;
const int COORD_Z_INDEX_FLUID = 2;

for( int i = 0; i < n_atoms_methane; i++ )
{
    coords_fluid[i][COORD_X_INDEX_FLUID] = x_point[i];
    coords_fluid[i][COORD_Y_INDEX_FLUID] = y_point[i];
    coords_fluid[i][COORD_Z_INDEX_FLUID] = z_point[i];
}

MOF5_coords(atom_types, sigma_point, epsilon_point, mass_point, atom_number, x_point, y_point, z_point, type_point);

double **coords_solid = new double*[n_atoms_MOF5];
for (int i = 0; i < n_atoms_MOF5; i++)
{
	coords_solid[i] = new double[N];
}

const int COORD_X_INDEX_SOLID = 0;
const int COORD_Y_INDEX_SOLID = 1;
const int COORD_Z_INDEX_SOLID = 2;

for( int i = 0; i < n_atoms_MOF5; i++ )
{
    coords_solid[i][COORD_X_INDEX_SOLID] = x_point[i];
    coords_solid[i][COORD_Y_INDEX_SOLID] = y_point[i];
    coords_solid[i][COORD_Z_INDEX_SOLID] = z_point[i];
}

}

Now this works fine if I use small values for n_atoms_MOF5, (like <2300), but in actuality there is over 3500. If I declare the correct n_atom_MOF5 I get an unhandled exception. Now to me that seems like not all of the atom positions are being saved in x, y and z_point, so when the program looks for the positions >2300 there is nothing there. I might be wrong though? What do you guys think?

[Imadatobanisa]

What's your algorithm? And (Input, and Output?)

[kg1992]

Hello, again.


This explains how to deal with exception:
http://www.cplusplus.com/doc/tutorial/exceptions/


If you have exception, you might want to check the "name" of the exception first, which provides a lot of hints for programmers to understand the situation.


What I'm guessing is that you actually allocated less memory somewhere, so you can read less x coordinates or something.


Darkmaster is absolutely right in this case. std::vector gives you everything you need. It tells how many elements in it, it checks if you are accessing wrong position for you, and it even clears its elements in it instead of you.


However, if you think std::vector is not what you need, following is my suggestion:

Check through all the constant variables you declared.
Check where they are used.
Make sure they are not deprecated.
Make sure your program does not use some deprecated variable somewhere.

I actually experienced the problem I described above.

My first trial:

// I need two consts
const int a = ?;
const int b = ?;

/* some codes using a */
/* some codes using b */

After some time passed:

// Hey, maybe I can just use b for everything.
const int a = ?;
const int b = ?;

/* some codes using a */ // Forget to change a to b !!
/* add more code on it but this time using b instead of a */

/* some codes using b */
/* add more code on it using b */

// This code still works anyway 

Then I changed "b" assuming that everything will change for me,
but there were still "a" standing somewhere cause problem, but not even compiler tells me why.