Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

unreachable code #7

Open
gabru-md opened this issue Sep 6, 2019 · 14 comments
Open

unreachable code #7

gabru-md opened this issue Sep 6, 2019 · 14 comments

Comments

@gabru-md
Copy link

gabru-md commented Sep 6, 2019

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
Copy link
Author

gabru-md commented Sep 6, 2019

cc : @LeeDoYup

@LeeDoYup
Copy link
Owner

LeeDoYup commented Sep 9, 2019

RobustSTL/RobustSTL.py

Lines 108 to 111 in 4185a8c

if trial != 1:
converge = check_converge_criteria(previous_remainders, remainders_hat)
if converge:
return [input, trends_hat, seasons_hat, remainders_hat]

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
Copy link
Author

gabru-md commented Sep 9, 2019

Yes this is correct, but if the solution does not converge ever, then the line at 117

return [input, trends_hat, seasons_hat, remainders_hat]

will never be executed since the loop starting at
while True:

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
Copy link
Author

gabru-md commented Sep 9, 2019

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

@LeeDoYup
Copy link
Owner

LeeDoYup commented Sep 9, 2019

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

@gabru-md
Copy link
Author

gabru-md commented Sep 9, 2019

yes it must be 👍

@LeeDoYup LeeDoYup reopened this Sep 11, 2019
@LeeDoYup
Copy link
Owner

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

@gabru-md
Copy link
Author

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

Cheers :)

@LeeDoYup
Copy link
Owner

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

@gabru-md
Copy link
Author

sure. Let me try !

@LeeDoYup
Copy link
Owner

LeeDoYup commented Feb 6, 2020

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
Copy link
Author

gabru-md commented Feb 12, 2020

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
Copy link
Owner

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

@iblasi
Copy link

iblasi commented Nov 28, 2020

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants