Remove dead code #257
Remove dead code #257
Conversation
|
Can one of the admins verify this patch? |
|
If you enable |
Yeah I believe we have to move certain code we have to move under DEBUG. I can comment inn details.
We have to fix that! I can help. We should have one of the travis build compiled with DEBUG enabled. |
|
We have to switch to spdlog and put |
1860dfd to
735bfa5
Compare
7792ce5 to
d13463b
Compare
|
Endly, I put only what cannot go inside Make a fine review please. |
Due to #if 0, if (0 && ...) and #if 1 ||
d13463b to
f7ddfba
Compare
ohm314
left a comment
There was a problem hiding this comment.
looks good, except for one function
coreneuron/permute/cellorder2.cpp
Outdated
| } | ||
|
|
||
| // size of groups with contiguous parents for each level | ||
| static void question(VVTN& levels) { |
There was a problem hiding this comment.
what's the use of an empty function?
There was a problem hiding this comment.
good question.
This function has been #if 0 since at least: 2016-10-05
There was a problem hiding this comment.
172 files changed, 18126 insertions(+), 7038 deletions(-)
It was an impressive commit by Pramod!
There was a problem hiding this comment.
It was an impressive commit by Pramod!
that was a gpu development branch being worked for a year or so and then it was merged together and thats why those stats.
This function has been #if 0 since at least: 2016-10-05
we need to discuss a bit about dead code aka #if 0. Lets take an example of :
void chklevel(VTN& level, size_t nident = 8) {
#if 0
nrn_assert(level.size() % nident == 0);
for (size_t i = 0; i < level.size(); ++i) {
size_t j = nident*int(i/nident);
nrn_assert(level[i]->hash == level[j]->hash);
}
#endif
}
In this case when we perform permutations/reordering of morphological trees, this routine is helpful to verify if identical sub-trees are ordered together. Ideally, there could be a flag/option to perform such checks at runtime. But, in the past CMake build or CLI11 options were minimal and hence such code was just added in #if 0 (and whenever you have to verify something, it was turned on).
For such cases, today one could add some build/cli option to turn on such checks that can be very helpful to identify performance/logical errors.
So I would suggest to review if there are any utility functions and how they can be incorporated rather than just removing it (I think there are few such cases in this PR).
There was a problem hiding this comment.
In this case we should introduce a macro _DEBUG_GPU and ifdef this. if 0 should always mean "delete me".
|
Sorry, I didnt get time to review this. I believe there are certain changes in this PR need to be handled differently. I will create separate ticket or PR to address that. |
* Remove dead code put in DEBUG what should be Due to #if 0, if (0 && ...) and #if 1 || * Remove empty function question CoreNEURON Repo SHA: BlueBrain/CoreNeuron@ca97c39
No description provided.