unreachable code

[gabru-md]

I was looking through the code base. I have an issue, can you please tell me whether this line is reachable or not? Since it is working in an infinite loop, so there must be a break statement so that the flow of control can go out of the infinite loop sequence, but I cannot find any such break in the code.

If I'm missing something, then can you please point it out to me.

Thanks a lot in advance.

Manish Devgan

[gabru-md]

cc : @LeeDoYup

[LeeDoYup]

https://github.com/LeeDoYup/RobustSTL/blob/4185a8cfb904b9b70f793460e6939b090d4edc2c/RobustSTL.py#L108-L111

In this part, check_converge_criteria method return True/False after comparing change of remainders. If converge_criteria are satisfied, the loop is break.

[gabru-md]

Yes this is correct, but if the solution does not converge ever, then the line at 117 https://github.com/LeeDoYup/RobustSTL/blob/4185a8cfb904b9b70f793460e6939b090d4edc2c/RobustSTL.py#L117 will never be executed since the loop starting at https://github.com/LeeDoYup/RobustSTL/blob/4185a8cfb904b9b70f793460e6939b090d4edc2c/RobustSTL.py#L89 is an infinite loop. therefore the solution must converge in order to return some result, else the loop will continue going on and on and will never exit

[gabru-md]

does this not mean that there is no need of the line at line number 117? because that becomes unreachable anyways.

[LeeDoYup]

Yes, you're right. The code needs to be correct for the unconverged situation.

[gabru-md]

yes it must be :+1:

[LeeDoYup]

Yes, I'll fix it. Of course, pull-request is always welcome !

[gabru-md]

what needs to be the change exactly? I'll be happy to make it for you.

Cheers :)

[LeeDoYup]

If you make pull-request about the infinite loop, i'll add comment about the codes and merge if it's okay.

[gabru-md]

sure. Let me try !

[LeeDoYup]

To close to the issue, i will fix it. I will add "trial limit" option and make a user able to select whether she/he run the codes until converge or set trial limit of iteration.

[gabru-md]

Hey @LeeDoYup I had to use stl in a c++ project and I therefore migrated your code to a c++ version at stl-cpp@gabru-md. Here I added the variable of MAX_TRIES which accounts to the maximum number of tries which are allowed before the process can be stopped. If this is something you'd like to have I'd be happy to provide you a PR for it.

[LeeDoYup]

I agree to set optional argument of the maximum number of iterations, and please make a PR.

[iblasi]

Looking to the code that @gabru-md shared, it is quite clear the required changes. I have added the max. iterations ("max_iter" parameter as it is usually used in scikit-learn package) as an input option, as it would help any user to define it if required. So the final function (I tested and it works properly on iterations) would be:

def _RobustSTL(input, season_len, reg1=10.0, reg2= 0.5, K=2, H=5, dn1=1., dn2=1., ds1=50., ds2=1., max_iter=50):
    '''
    args:
    - reg1: first order regularization parameter for trend extraction
    - reg2: second order regularization parameter for trend extraction
    - K: number of past season samples in seasonaility extraction
    - H: number of neighborhood in seasonality extraction
    - dn1, dn2 : hyperparameter of bilateral filter in denoising step.
    - ds1, ds2 : hypterparameter of bilarteral filter in seasonality extraction step.
    - max_iter : maximum iterations to converge
    '''
    sample = input
    trial = 1
    patient=0
    while(trial <= max_iter):

        print("[!]", trial, "iteration will start")

        #step1: remove noise in input via bilateral filtering
        denoise_sample =\
                denoise_step(sample, H, dn1, dn2)

        #step2: trend extraction via LAD loss regression 
        detrend_sample, relative_trends =\
                trend_extraction(denoise_sample, season_len, reg1, reg2)

        #step3: seasonality extraction via non-local seasonal filtering
        seasons_tilda =\
                seasonality_extraction(detrend_sample, season_len, K, H, ds1, ds2)

        #step4: adjustment of trend and season
        trends_hat, seasons_hat, remainders_hat =\
                adjustment(sample, relative_trends, seasons_tilda, season_len)

        #step5: repreat step1 - step4 until remainders are converged
        if trial != 1:            
            converge = check_converge_criteria(previous_remainders, remainders_hat)
            if converge:
                print("[!] RobustSTL completed in", trial, "trials!")
                return [input, trends_hat, seasons_hat, remainders_hat]

        trial+=1

        previous_remainders = remainders_hat[:]
        sample = trends_hat + seasons_hat + remainders_hat

    print("[!] RobustSTL forces to and end!")
    return [input, trends_hat, seasons_hat, remainders_hat]

def RobustSTL(input, season_len, reg1=10.0, reg2= 0.5, K=2, H=5, dn1=1., dn2=1., ds1=50., ds2=1., max_iter=50):
    if np.ndim(input) < 2:
        return _RobustSTL(input, season_len, reg1, reg2, K, H, dn1, dn2, ds1, ds2, max_iter)

    elif np.ndim(input)==2 and np.shape(input)[1] ==1:
        return _RobustSTL(input[:,0], season_len, reg1, reg2, K, H, dn1, dn2, ds1, ds2, max_iter)

    elif np.ndim(input)==2 or np.ndim(input)==3:
        if np.ndim(input)==3 and np.shape(input)[2] > 1:
            print("[!] Valid input series shape: [# of Series, # of Time Steps] or [# of series, # of Time Steps, 1]")
            raise
        elif np.ndim(input)==3:
            input = input[:,:,0]
        num_series = np.shape(input)[0]

        input_list = [input[i,:] for i in range(num_series)]

        from pathos.multiprocessing import ProcessingPool as Pool
        p = Pool(num_series)
        def run_RobustSTL(_input):
            return _RobustSTL(_input, season_len, reg1, reg2, K, H, dn1, dn2, ds1, ds2, max_iter)
        result = p.map(run_RobustSTL, input_list)

        return result
    else:
        print("[!] input series error")
        raise