arminalaghi / scsynth

Synthesis tool for stochastic computing
MIT License
21 stars 11 forks source link

Code revision aimed towards Multivariate use #1

Closed danilo-bc closed 5 years ago

danilo-bc commented 5 years ago

The code seems to now succesfully generate Verilog files for given functions. Tested in Octave 4.2.2

Core functionality has remained unchanged.

There seems to be some issue from older functions having "singleWeightLFSR" boolean and newer versions having strings "ConstantRNG='SharedLFSR'" and "InputRNG='LFSR'"

arminalaghi commented 5 years ago

Hi danilo-bc,

Thanks for contribution. Please allow me some time to review and test the code.

Cheers, Armin

arminalaghi commented 5 years ago

Finally getting to this. Sorry for delay. Can you explain why the "parallel (3.1.3)" package is needed? Is it for speedup? Can we do without it? or can you make it optional?

danilo-bc commented 5 years ago

Hello, thank you for maintaining this repository.

I noticed the algorithm struggles to complete basic multivariate synthesis tasks, such as x1*x2, and it doesn't use proper resources.

I'm running an 8-thread Ryzen 2400G and performance is usually capped at 1 core, 2 threads.

I started using the parallel package with the intention of speeding up multiple optimization tasks using all available CPU cores (disregarding possible GPU speedups for the moment).

There doesn't seem to be any problem with having the parallel package, other than having to download an extra package as dependency.

If necessary, I can comment out the parallel lines or create a parallel branch altogether. Another possible solution would be to add an extra argument 'Parallel' defaulting false to the functions where I introduced the package.

arminalaghi commented 5 years ago

Thank you for contributing to this repo.

If possible, add your last suggestion: add a parallel parameter that defaults to false. It would be nice to have the code run out of the box on octave. Also I am not sure how this behaves on Matlab. We can add a comment that mentions your observations about the speed and how the parallel package can speed things up.

Cheers

danilo-bc commented 5 years ago

The main feature that drew me to this repository was STRAUSS. Unfortunately, I found out it still lacks a multivariate implementation.

What needs to be done to implement STRAUSS in a multivariate fashion? Could you please provide some guidelines? (Either in text or commenting source code/writting pseudocode and uploading to git)

I believe the whole process of synthesis is also generally faster in more accurate in STRAUSS than on MReSC (I'm a bit unsure about this one), so implementing it would also possibly solve a few problems.

danilo-bc commented 5 years ago

Hi, how is the revision going?

arminalaghi commented 5 years ago

A couple of things:

  1. I still couldn't get around the parallel package error. Even though it is inside an if statement, octave still complains. I did a quick search but didn't find a good solution. We need something like #ifdef in C++ to make sure octave never reads the lines related to the parallel package.

  2. I updated my octave and installed the parallel package. But now I am getting an error from a piece of code that already existed in the repo. Here are the commands I am running: func = @sin; VerilogSCFromFunction(func, 3, 4, 4, 4, 'name', singleWeightLFSR=true, ConstantRNG='SharedLFSR', InputRNG='LFSR', ConstantSNG='HardWire', InputSNG='Comparator', SCModule='ReSC', domain=[0,1], granularity=100)

I get: error: VerilogSCFromFunction: operator /: nonconformant arguments (op1 is 1x1, op2 is 1x2) error: called from VerilogSCFromFunction at line 107 column 5

Which is this line: x = [domain(1):(domain(2) - domain(1))/granularity:domain(2)];

Any ideas why this is giving me an error?

arminalaghi commented 5 years ago

Regarding the multivariate implementation of STRAUSS: The main difference between STRAUSS and ReSC is that STRAUSS allows "asymmetric" coefficients. For example, imagine a singe variable case with degree 3, and assume the three inputs are x1 x2 x3. ReSC uses the same coefficient if [x1 x2 x3] = [1 0 0] or [0 1 0] or [0 0 1], but STRAUSS allows these input combinations to have different coefficients. However, finding the best coefficient combination requires a search in a space the quickly explodes if the degree increases.

The problem gets even worse when you switch to multi-variate. Multi-variate ReSC is already slow, and adding the STRAUSS option to it makes it even slower, so I decided not to add it.

One last note: STRAUSS also has a constant generation routine that can be used in ReSC as well. I believe the current code uses the constant generation routine, even if you call ReSC functions.

danilo-bc commented 5 years ago

Regarding the parallel package error, I uninstalled my package, leaving me with io, optim, statistics and struct. My Octave is 4.4.1. I tried to run MReSC and ReSC from function and both only gave me a warning, probably an old package conflicting with some function upstreamed to Octave's built-in library. warning: function /home/dan/octave/statistics-1.4.0/base/mad.m shadows a core library function

I took at look at your code snippet and your function seems different from the one I have. With my added 'useParallel' I have the following:

function VerilogSCFromFunction (func, degree, N, m_input, m_coeff,...
                                  nameSuffix, ConstantRNG='SharedLFSR',...
                                  InputRNG='LFSR', ConstantSNG='HardWire',...
                                  InputSNG='Comparator', SCModule='ReSC',...
                                  domain = [0, 1], granularity=100,...
                                  useParallel=false);

This means there's an extra singleWeightLFSR=true parameter, making granularity assume the place of "useParallel". Which gives you the load pkg error.

I pointed the difference in VerilogSCFromFuction and VerilogMReSCFromFunction in my very first post, although I wasn't clear on the files:

There seems to be some issue from older functions having "singleWeightLFSR" boolean and newer versions having strings "ConstantRNG='SharedLFSR'" and "InputRNG='LFSR'"

Taking away that phantom option executes your given sample, producing 'name' Verilog files.

Another thing to keep in mind is that Matlab doesn't support these variable='value' assignments on function calls, so I would recommend a structure similar to this, which should work for both programs (aside from the small optim package options that need to be manually handled):

func = @sin;
ConstantRNG='SharedLFSR';
InputRNG='LFSR';
ConstantSNG='HardWire';
InputSNG='Comparator';
SCModule='ReSC'; 
domain=[0,1];
granularity=100;

VerilogSCFromFunction(func, 3, 4, 4, 4, 'name', ...
                                       ConstantRNG, ...
                                       InputRNG, ConstantSNG, InputSNG, ...
                                       SCModule, domain, granularity);
arminalaghi commented 5 years ago

Thanks for clarifying this. I'll try this tomorrow and get back to you.

danilo-bc commented 5 years ago

One last note: STRAUSS also has a constant generation routine that can be used in ReSC as well. I believe the current code uses the constant generation routine, even if you call ReSC functions.

Just forgot to mention this part isn't done for MReSC versus ReSC, so it's possibly my next contribution to the repository. I saw the differences in the files, but wasn't clarified if it was intended or not.

arminalaghi commented 5 years ago

You are right! The MReSC version does not use the constant generation procedure.

Ok. I tested it with parallel package installed and it seems to be working.

danilo-bc commented 5 years ago

I tested it with parallel package installed and it seems to be working.

Just a final remark: the parallel package is entirely optional.

The tests I made earlier were all after uninstalling the package and testing. What happened is that MReSC and ReSC generators have different function calls, and you had mixed 1 MReSC parameter inside, that possibly resulted in Octave interpreting that last one as 'useParallel=1'.