New answers tagged

2

Assigning number of processors You currently set procs = 3, specifically for your 4-core machine. You can get the number of (logical) cores of your machine using the multiprocessing.cpu_count function: from multiprocessing import Pool, cpu_count procs = cpu_count() - 1 Create Pool just once Rather than creating a new Pool every iteration of your for-loop,...


1

Avoid Using Namespace std It is possible that #include <bits/stdc++.h> and using namespace std; was provided by the challenge site, in that case it might be better to delete that from the code when you answer the challenge. If you are coding professionally you probably should get out of the habit of using the using namespace std; statement. The code ...


1

In addition to G. Sliepen’s idea, you could use the STL’s std::valarray<double>. This would let you replace something like for (int d = 0; d < DIM; ++d) { p->x[d] += dt * (p->v[d] + a * p->F[d]); p->F_old[d] = p->F[d]; } with something like p->F_old = p->F; p->x += dt * (p->v + a * p->F); It would also be ...


3

Use a vector math library Find a suitable library that implements coordinate vectors, so you don't have to implement them as arrays of doubles. Ideally your struct Particle should look like: struct Particle { double m; // mass vec3 x; // position vec3 v; // velocity vec3 F; // force vec3 F_old; // force past time step }; ...


1

I will focus on one thing already addressed by another answer but that I think deserve more attention: the Single Responsibility Principle. Your NBody class has several functionalities merged into one, which would be advisable to separate. It, as far as I can see: it represent a group of N particles it provides the algorithm to perform the physics ...


3

I can't comment due to being new here, but Roland Illig's assertion that it should be ++step and not step++ and that it shows that you do not understand C++ is incorrect. In C++, the position of the ++ determines the order of how the expression is evaluated. So in ++step, the variable is incremented before any action with it is performed, while in step++, ...


10

First Of All You are doing a great job as a beginner. I have been programming for 10 years and my code for a long time was much, much less readable that what you have written. That said: What needs Fixing I'm not privy of all of the details of the n-body problem but I have an idea of what it does. I'm not an expert on numerical accuracy so I won't comment ...


8

In addition to the other answers: The init_data function does not belong in the Nbody class. Nowhere in the definition of the N-body problem will you find the word "random", and using random input data is only connected to your particular situation, therefore this code should be moved into main.cpp. In the constructor of Nbody, there's no need for the ...


10

In addition to the other answers: Use unsigned integer type for DIM, Nbody.step, and Nbody.n since none of this can be negative; Use constexprsince C++11 instead just const for both DIM and EPS; Get rid of the unused argc and argv arguments in main; Consider more usage of const. For example f in Nbody::force() can be const, and a in Nbody::update_position ...


25

The Great! You're not making the basic beginner error of using using namespace std;! The main() function is only 3 lines of code. The function declarations in the nbody class that don't change things include const which will help optimization later. The code utilizes the C++ random number generation rather than the C srand() and rand() functions. Because ...


10

Your code is written in a hybrid C/C++ style. For instance your destructor has a delete (I can't find where the corresponding new is) and that is basically never needed. Use a std::vector to store array-like data. Also you do a lot of parameter passing like void Nbody::update_position(Particle *p). Use references instead, and use const Particle &p if ...


Top 50 recent answers are included