NCNN optimizer: wrong assignment/test will corrupt a layer

[adegerard]

I was doing some code review... (main branch)

https://github.com/chaiNNer-org/chaiNNer/blob/9dac4fc7b6bf9934cf8a009c6c7b456990fef827/backend/src/nodes/impl/ncnn/optimizer.py#L658

    def __eliminate_dropout ....
    ....
                j = i - 1
                for j in range(i - 1, -1, -1):
                    ...
                else:
                    j -= 1
                if j == -1:
                    continue
                self.model.layers[j].outputs[0] = layer.outputs[0]

NCNN src code: https://github.com/Tencent/ncnn/blob/master/tools/ncnnoptimize.cpp#L2050

Issue If i == 0, then layers[-2] is corrupted Same problem in __eliminate_pooling1x1, __eliminate_noop, __eliminate_split functions

How to solve Remove the else block

How to reproduce I was not able to reproduce because I didn't run tests (had no model)

-> I can create a PR with this solution without testing, let me know if this is ok with you Anyway, I'll test it somehow later if I find a model for this: I'm reusing this code and I plan to test a lot of models...

[adegerard]

Moreover, it seems that there are bugs in loop: in c language:

for (0; j < layer_count; j++)
   ...

after this loop, j = layer_count

in python:

for j in range(0, layer_count):
     ...

after this loop, j = layer_count -1

[joeyballentine]

@theflyingzamboni can you please check this?

Thanks for reporting

[theflyingzamboni]

@adegerard Go ahead and do that fix for the first one. I had assumed that if range start == range stop that the value would never decrement in the loop and it would go immediately to the else, but apparently this isn't true. Not sure why, the stop argument in range is exclusive.

For your second post, are you saying that there's something wrong with how python is handling that loop? I don't see that code in the optimizer anywhere.

[theflyingzamboni]

I see what you mean, it looks like the way python does for...range loops, it evaluates whether to break the loop prior to incrementing the variable, so j never ticks up to the stop value like it would in a C-like language. That could be causing a number of potential bugs, I don't recall at this point.

[adegerard]

I see what you mean, it looks like the way python does for...range loops, it evaluates whether to break the loop prior to incrementing the variable, so j never ticks up to the stop value like it would in a C-like language. That could be causing a number of potential bugs, I don't recall at this point.

exactlty: potential bugs as you said and I agree: Because I currently found a single model only which is "optimized" and optimized by only one function... -> it's not an urgent issue to fix and complicated to validate because I did not found any model for code coverage.

[theflyingzamboni]

and complicated to validate because I did not found any model for code coverage.

This is why most of the code I translated for that is untested lol. I never could find models to test the vast majority of the functions.