nengo / nengo-extras

Extra utilities and add-ons for Nengo
https://www.nengo.ai/nengo-extras
Other
5 stars 8 forks source link

Added function to generate triangular intercepts #91

Closed p3jawors closed 4 years ago

p3jawors commented 4 years ago
hunse commented 4 years ago

These should go in nengo_extras/dists.py and the tests should go in test_dists.py. Also, the import scipy.special should be in the functions, so that we don't require it to import the file (you can see how it's done in MultivariateCopula, where we check it can be imported in the constructor to fail fast if it's not available, and then import it again in the generate function where it's actually needed).

p3jawors commented 4 years ago

I can move it over to the dists.py, but isn't it pylint standard to have all imports at the top? We've been moving all our imports in the control repos to the top to avoid travis ci errors. That being said, I can definitely move that over if that's what you guys want in nengo land :+1:

hunse commented 4 years ago

Yes, pylint likes that but there are reasons to not do it that way (like this situation). The reason that rule exists is to fail as fast as possible if the requested package isn't installed, but in this case we only want to fail if the user is making something that actually needs that package. So you just need to disable that pylint error for those lines.

The other way to do it is to have e.g. a compat.py file that tries to get all the imports we need. So it'd have something like

try: 
    import scipy
except: 
    scipy = None

and then you can from compat import scipy at the top of the file, and in your distribution that needs scipy you can do assert scipy is not None. The downside to that is that if scipy is there, you import it even if the user isn't doing something that needs it. This extra import can slow things down (I find this especially significant for TensorFlow, which can take a few seconds to import).

p3jawors commented 4 years ago

Moved the classes and tests into the dists and test_dists scripts, ready for review

studywolf commented 4 years ago

Closing this PR, desired functionality can be implemented in a couple lines with nengo.dists.CosineSimilarity and np.random.triangular

            triangular = np.random.triangular(left, mode, right, size=n_neuron)
            intercepts = nengo.dists.CosineSimilarity(dimensions + 2).ppf(1 - triangular)