Update GunnsRosesTiming.cpp - memory management and the use of null p… #131
Conversation
…ointers Replacing manual memory management with std::unique_ptr for automatic deallocation, eliminating unnecessary null pointer checks. A division-by-zero risk in update() was also addressed by adding a safeguard.
There was a problem hiding this comment.
Thanks for the submission! I have some changes & questions, please address my review comments and resubmit. See the submission guidelines wiki: https://github.com/nasa/gunns/wiki/Open-Source-Submission-Guidelines#how-to-submit-changes. Please reference issue #132 in your commit messages. Thanks!
| if (N_STEPS <= step) { | ||
| storePotentials(); | ||
| // dumpA(); |
There was a problem hiding this comment.
I want to keep these 2 'dump' calls in, and commented out, so that we can uncomment them for debugging. Please restore these lines and resubmit. If you want, you can add a comment like "these are for debugging, uncomment as needed", etc.
| if (errCheck) { | ||
| double error = 0.0; | ||
| for (unsigned int i=0; i<5; ++i) { | ||
| error += std::fabs(gpuPotentials[i] - cpuPotentials[i]); |
There was a problem hiding this comment.
Please change to std::abs and resubmit. We're trying to adhere to a new coding standard that explicitly uses the standard library namespace for all such math functions.
| if (errCheck) { | ||
| double error = 0.0; | ||
| for (unsigned int i=0; i<5; ++i) { | ||
| error += std::fabs(gpuSparsePotentials[i] - cpuPotentials[i]); |
There was a problem hiding this comment.
Please change to std::abs and resubmit. We're trying to adhere to a new coding standard that explicitly uses the standard library namespace for all such math functions.
| printf("\nTerminating sim:\n"); | ||
| exec_terminate("", ""); // from Trick's exec_proto.h, tell Trick to kill the sim |
There was a problem hiding this comment.
We need this to terminate the sim. Please restore this line and resubmit.
| printf("\nTerminating sim:\n"); | ||
| exec_terminate("", ""); // from Trick's exec_proto.h, tell Trick to kill the sim | ||
| delete network; // Deallocate the network | ||
| step = 0; |
There was a problem hiding this comment.
What is the purpose of these additions 169 - 176? We are terminating the sim, nothing more needs to be done.
Replacing manual memory management with std::unique_ptr for automatic deallocation, eliminating unnecessary null pointer checks. A division-by-zero risk in update() was also addressed by adding a safeguard.