LBHB / NEMS0

THIS VERSION OF NEMS IS NO LONGER SUPPORTED. PLEASE USE THE NEW NEMS REPOSITORY OR INSTALL NEMS_DB TO GET NEMS0 SUPPORT.
GNU General Public License v3.0
8 stars 4 forks source link

Added new epoch function and fixed unit tests #109

Closed bburan closed 6 years ago

bburan commented 6 years ago

New epoch function also has a unit test.

svdavid commented 6 years ago

@jacobpennington pytest is failing (among other things) on test_bounds.test_scalar_bounds(). It seems that the bounds returned by the function are not in the expected order. This makes me wonder if/how you are making sure that the bounds vector is ordered to match the phi vector. Can you confirm that bound and phi are mapping in the same order? If so, I think that requires updating this test code.

bburan commented 6 years ago

It seems Travis is happy now. Thanks @svdavid

svdavid commented 6 years ago

Hmm. It should still fail on the bounds_test. Maybe there's some nonspecificity/randomness in the order with which phi (or bounds) values get mapping into sigma vectors? If the order is fixed, then the bounds vector should be the same. What happens for me is the bounds vector comes out in a different order than the unit test expects. Perhaps this didn't happen for Travis? @bburan do you have a sense of how this should work? We can force it to be alphabetical maybe?

bburan commented 6 years ago

Can you paste the output of your test? It passes on my computer and Travis also reports it passes (https://travis-ci.com/LBHB/NEMS/jobs/128033318). If dictionaries are involved and you use Python 3.5, that's the likely culprit (Python 3.6 now guarantees that dictionaries return their keys in the order they were inserted, whereas earlier versions of Python did not make that guarantee).


From: Stephen D [notifications@github.com] Sent: Thursday, June 07, 2018 12:45 PM To: LBHB/NEMS Cc: Brad Buran; Mention Subject: Re: [LBHB/NEMS] Added new epoch function and fixed unit tests (#109)

Hmm. It should still fail on the bounds_test. Maybe there's some nospecificity in the order with which phi (or bounds) values get mapping into sigma vectors? If the order is fixed, then the bounds vector should be the same. what happens for me is the bounds vector comes out in a different order than the unit test expects. @bburanhttps://github.com/bburan do you have a sense of how this should work? We can force it to be alphabetical maybe?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/LBHB/NEMS/pull/109#issuecomment-395542077, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AAT5QnBdyzr1wgD6aiR7IdN0u4-lxkI4ks5t6YLTgaJpZM4UdNDY.

svdavid commented 6 years ago

This is what I get. assert bounds_vector returns a list of the correct tuples, but in the wrong order.


modelspec = [{'fn': 'nems.modules.weight_channels.gaussian', 'fn_kwargs': {'i': 'stim', 'n_chan_in': 18, 'o': 'pred'}, 'phi': {'me...mplitude': 2.071742144205505, 'base': -0.3911174879125417, 'kappa': 0.4119910862820081, 'shift': 0.34811284718401403}}]

    def test_scalar_bounds(modelspec):
        modelspec[1]['bounds'] = {
                'amplitude': (0, 5),
                'base': (None, None),
                'kappa': (-1, None),
                'shift': (None, 5)
                }
>       assert bounds_vector([modelspec[1]]) == [
                (0, 5), (None, None), (-1, None), (None, 5)
                ]
E       assert [(-1, None), ... (None, None)] == [(0, 5), (None...e), (None, 5)]
E         At index 0 diff: (-1, None) != (0, 5)
E         Use -v to get the full diff

test_bounds.py:46: AssertionError
bburan commented 6 years ago

@jacobpennington - I think this is due to the fact that Python 3.6 guarantees that values in a dictionary are returned in the order they are added. However, depending on this behavior is risky because we need to ensure that the phi and bounds vectors are the same order.

Can you generalize the code for phi2vector so it works in 2D as well?


From: Stephen D [notifications@github.com] Sent: Thursday, June 07, 2018 1:17 PM To: LBHB/NEMS Cc: Brad Buran; Mention Subject: Re: [LBHB/NEMS] Added new epoch function and fixed unit tests (#109)

This is what I get. assert bounds_vector returns a list of the correct tuples, but in the wrong order.

modelspec = [{'fn': 'nems.modules.weight_channels.gaussian', 'fn_kwargs': {'i': 'stim', 'n_chan_in': 18, 'o': 'pred'}, 'phi': {'me...mplitude': 2.071742144205505, 'base': -0.3911174879125417, 'kappa': 0.4119910862820081, 'shift': 0.34811284718401403}}]

def test_scalar_bounds(modelspec):
    modelspec[1]['bounds'] = {
            'amplitude': (0, 5),
            'base': (None, None),
            'kappa': (-1, None),
            'shift': (None, 5)
            }
  assert bounds_vector([modelspec[1]]) == [

(0, 5), (None, None), (-1, None), (None, 5) ] E assert [(-1, None), ... (None, None)] == [(0, 5), (None...e), (None, 5)] E At index 0 diff: (-1, None) != (0, 5) E Use -v to get the full diff

test_bounds.py:46: AssertionError

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/LBHB/NEMS/pull/109#issuecomment-395550947, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AAT5QgH-N7W96zPZ8m3acxRHFtUjLGHTks5t6YpHgaJpZM4UdNDY.

bburan commented 6 years ago

@svdavid What's keeping you from accepting this pull request?

svdavid commented 6 years ago

looks like some conflicts. Let me see what's happening.