Various small issues - Githubissues

casperkaae / parmesan

Variational and semi-supervised neural network toppings for Lasagne

Other

208 stars 31 forks source link

Various small issues #29

Closed wuaalb closed 8 years ago

wuaalb commented 8 years ago

I ran into a couple of small issues trying to use Parmesan

The recently added datasets add a whole bunch of dependencies (nltk, sklearn, scipy, ..); would it be possible to import those packages locally for the functions that need them? Now to use Parmesan you need all that stuff installed even if you don't use those datasets.
Would it be possible to change _srng in SimpleSampleLayer and SampleLayer to srng? The leading underscore implies it's a private variable (although Python doesn't enforce this). I find using a fixed seed before, for instance, approximating log likelihood by importance sampling useful for exactly reproducing results.
The git has some binary files that probably shouldn't be there parmesan/dist/Parmesan-0.1.dev1-py2.7.egg, parmesan/misc/eval_L5000.jpg.

casperkaae commented 8 years ago

The recently added datasets add a whole bunch of dependencies (nltk, sklearn, scipy, ..); would it be possible to import those packages locally for the functions that need them? Now to use Parmesan you need all that stuff installed even if you don't use those datasets.

Yes I agree, Just did't though about it when i added them since I have the toolkits. I'll move the imports into the functions.

Would it be possible to change _srng in SimpleSampleLayer and SampleLayer to srng? The leading underscore implies it's a private variable (although Python doesn't enforce this). I find using a fixed seed before, for instance, approximating log likelihood by importance sampling useful for exactly reproducing results.

I prefer to stick with the "_srng" notation since it corresponds to the Lasagne?

The git has some binary files that probably shouldn't be there parmesan/dist/Parmesan-0.1.dev1-py2.7.egg, parmesan/misc/eval_L5000.jpg.

At least the dist folder seems to be an error :) Maybe I should also find a better place to put the images...

wuaalb commented 8 years ago

Ok great.

If you want to keep _srng, how about adding a

def seed(self, seed=None):
    self._srng.seed(seed)

to SimpleSampleLayer and SampleLayer?

I think the misc folder for the plot is fine (although I would just put it in the root); what I meant was that there's a .jpg and .png of the same plot, but only the .png is used in the readme.md.

casperkaae commented 8 years ago

Ok, will PR #31 solve your issue? Otherwise you might edit it so it does?