New method rbd fast

[celliern]

Hello !

I have implemented the RBD-FAST algorithm following the SALib contributor guide. I have updated analyzer, examples, docs, examples and tests. The CLI is working. We use the LHS sampler, already implemented in SALib.

I also fixed a little mistake on the latin sampler CLI : the args were parsed before the nsampler arg addition.

I only did the programming work here : many thanks to J. Goffart and M. Rabouille for the matlab implementation and S. Juricic for a first python implementation.

Nicolas Cellier

[celliern]

Hm, I don's know why this test fail. It's related to Morris, and it do not fail when it's the only test running. Should not be related to my change ?

[jdherman]

This is great!! Examples and tests too. Thank you.

Let me try to figure out what's happening with Travis. I can see that the Morris problem should have nothing to do with the new RBD-fast method.

The Python 2 test is failing because of something to do with unicode characters, would you mind looking for this?

==================================== ERRORS ====================================
__________________ ERROR collecting tests/test_regression.py ___________________
../../../miniconda/envs/testenv/lib/python2.7/site-packages/_pytest/python.py:403: in _importtestmodule
    mod = self.fspath.pyimport(ensuresyspath=importmode)
../../../miniconda/envs/testenv/lib/python2.7/site-packages/py/_path/local.py:668: in pyimport
    __import__(modname)
../../../miniconda/envs/testenv/lib/python2.7/site-packages/_pytest/assertion/rewrite.py:213: in load_module
    py.builtin.exec_(co, mod.__dict__)
tests/test_regression.py:8: in <module>
    from SALib.analyze import rbd_fast
E     File "/home/travis/build/SALib/SALib/SALib/analyze/rbd_fast.py", line 43
E   SyntaxError: Non-ASCII character '\xc3' in file /home/travis/build/SALib/SALib/SALib/analyze/rbd_fast.py on line 44, but no encoding declared; see http://python.org/dev/peps/pep-0263/ for details
!!!!!!!!!!!!!!!!!!! Interrupted: 1 errors during collection !!!!!!!!!!!!!!!!!!!!
=========================== 1 error in 17.53 seconds ===========================

The Python 3 test is the Morris error, which I'm not sure how to address. @willu47 does any of this ring a bell?

=================================== FAILURES ===================================
____________ TestLocallyOptimalStrategy.test_local_optimised_groups ____________
self = <test_morris_strategies.TestLocallyOptimalStrategy object at 0x7fb5afcf2e48>
setup_param_groups_prime = '/tmp/tmp9c_y0inp'
    def test_local_optimised_groups(self,
                                    setup_param_groups_prime):
        """
            Tests that the local optimisation problem gives
            the same answer as the brute force problem
            (for small values of `k_choices` and `N`)
            with groups
            """
        N = 8
        param_file = setup_param_groups_prime
        problem = read_param_file(param_file)
        num_levels = 4
        grid_jump = num_levels / 2
        k_choices = 4
    
        num_params = problem['num_vars']
    
        num_groups = len(set(problem['groups']))
    
        input_sample = _sample_groups(problem, N, num_levels, grid_jump)
    
        strategy = LocalOptimisation()
    
        # From local optimal trajectories
        actual = strategy.find_local_maximum(input_sample, N, num_params,
                                             k_choices, num_groups)
    
        brute = BruteForce()
        desired = brute.brute_force_most_distant(input_sample,
                                                 N,
                                                 num_params,
                                                 k_choices,
                                                 num_groups)
>       assert_equal(actual, desired)
E       AssertionError: 
E       Items are not equal:
E       item=0
E       
E        ACTUAL: 1
E        DESIRED: 0
tests/sample/morris/test_morris_strategies.py:223: AssertionError
=============== 1 failed, 96 passed, 6 skipped in 138.44 seconds ===============

[celliern]

I'll deal with the py2 error : I try to add the proper shebang, and if it's not enough I will eliminate the unicode characters (joy of the international scientific community, we have accents in our names ;))

[celliern]

The local tests show now the same error on py2 and py3, linked to TestLocallyOptimalStrategy.

[jdherman]

Thanks! I can't seem to figure out why this test is failing. If there really is a bug, how did it ever pass before?

I am not an expert on the local optimization Morris method being tested here. As I understand it's are both looking for a subset of input samples that are maximally far apart, which should match the brute-force calculation. In the simple test case (32 samples x 7 parameters), these are the indices found:

Actual: [1, 3, 5, 7]
Desired: [0, 1, 3, 7]

Which correspond to these input samples:

ACTUAL 
[[ 0.33333333  0.66666667  0.          1.          0.33333333  0.  0.33333333]
 [ 1.          0.          0.66666667  1.          0.33333333  0.  0.33333333]
 [ 1.          0.66666667  0.33333333  0.          0.66666667  1.          1.        ]
 [ 0.33333333  0.66666667  0.33333333  0.66666667  0.          0.33333333  0.33333333]]

DESIRED 
[[ 0.33333333  0.66666667  0.          0.33333333  1.          0.66666667   1.    ]
 [ 0.33333333  0.66666667  0.          1.          0.33333333  0.    0.33333333]
 [ 1.          0.          0.66666667  1.          0.33333333  0.    0.33333333]
 [ 0.33333333  0.66666667  0.33333333  0.66666667  0.          0.33333333    0.33333333]]

They should return the same thing, of course. I'm afraid we'll have to hunt this down before we can merge the PR (even though it has nothing to do with the PR). Thanks again @celliern for the great contribution, and for your patience as we try to figure this out.

Edit: I also see that the preceding test, which covers the same functions but without groups, passed. So it seems likely that the problem has something to do with the groups feature.

tests/sample/morris/test_morris_strategies.py::TestLocallyOptimalStrategy::test_find_local_maximum_distance PASSED [ 88%]
tests/sample/morris/test_morris_strategies.py::TestLocallyOptimalStrategy::test_local_optimised_groups FAILED [ 89%]

[celliern]

No problem : I may try to figure out that bug at the same time if you want.

[jdherman]

Sure, if you have any ideas, please go for it! I was looking around this morning and didn't manage to find the problem.

[celliern]

tests all pass if I withdraw the regression test for rbd-fast. Why? I have no clue...

[jdherman]

Wow, cryptic. I can confirm. And it gets worse ... inside the test for rbd-fast, if you change the sampling method from latin to saltelli ... then the other Morris test passes.

If you keep latin and change the number of samples from 10,000 to 20,000 ... then the Morris test also passes.

It must have something to do with modifying the global state of the random number generator(???) Still, the Morris test should not be failing under any condition. Looks like a stochastic test failure.

What options are there? specify random seed for every test? (several of the regression tests do this already).

[celliern]

Shouldn't be better to understand why the test fail under some random seeds?  If the test pass only in certain seed, it's not good and can be happening in real case, no? Le 19 janv. 2018 20:44, à 20:44, Jon Herman <notifications@github.com> a écrit:

…

Wow, cryptic. I can confirm. And it gets worse ... inside the test for rbd-fast, if you change the sampling method from `latin` to `saltelli` ... then the other Morris test passes. If you keep `latin` and change the number of samples from 10,000 to 20,000 ... then the Morris test also passes. It must have something to do with modifying the global state of the random number generator(???) Still, the Morris test should not be failing under any condition. Looks like a stochastic test failure. What options are there? specify random seed for every test? (several of the regression tests do this already). -- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: #186 (comment)

[jdherman]

Yes, that's the much smarter option ... I'll try to track down which variables inside that test are different between the success and failure cases.

Update: the issue in the failure case is that the brute method identifies one trajectory with a larger distance that the local method fails to find (it reports the trajectory with the second-largest distance instead).

The input samples are the same, and the distance matrices calculated for the two cases are the same. So, I believe the problem is somewhere in the local method, specifically in this loop:
https://github.com/SALib/SALib/blob/master/SALib/sample/morris/local.py#L51

Unfortunately I haven't had much luck tracking it beyond this, and need to set this aside for a while.

[celliern]

Lucky me, I wanted to add a method, I found the most cryptic bug I have encountered in my (short) career.

I propose to find how to reproduce the bug, create an issue and make an other PR to deal with it. After (and before) solving it, going back to this PR and merge it.

[jdherman]

Ok, after some time I'm not convinced this is a bug, instead I think we need to change our tests. We should not be expecting the local optimization to match the globally optimal trajectory every time. See #188 .

I would be ok with commenting out the failing test in order to merge this new RBD-FAST method, and then dealing with the test changes as soon as possible in a different PR.

[celliern]

I have commented the failing tests (TestLocallyOptimalStrategy.test_find_local_maximum_distance and TestLocallyOptimalStrategy.test_local_optimised_groups).
Should be working now !

[jdherman]

Ok let's do it. Thank you!! We'll take care of fixing the tests over in #188 .

I will try to get this out in a new release soon.
Edit: Now released in v1.1.3 on PyPI. Thanks again!