danpovey/lilcom

Make sure reconstruction test is runnable end to end

Opened this issue · 8 comments

danpovey commented

@soroushzargar perhaps you could do this.. it would be better if the reconstruction-test were easier to run. For example, have a script that downloads and suitably formats the appropriate data and then runs the test. Right now there is some hacky stuff going on with a temp dir that probably isn't in the source tarball.

I was only printing out the bits in the file as a debugging step. But I think we should be displaying the file sizes or bits per unit time in some form alongside the various compression methods; this will make comparisons more meaningful.

I just set the bits per sample to 6 in reconstruction-test.py but we may want to test various settings. What I would really like is some info about PSNR's and file sizes formatted in a way that's easily readable. IMO a simple Python program that prints out a summary would be preferable to this CSV stuff, in that it would be clearer. But as long as you do print out something readable at some point (maybe as a post processing step that might be in a separate script) it's OK with me. Make sure it's all documented with a top-level run.sh or something.

soroushzargar commented

@soroushzargar perhaps you could do this.. it would be better if the reconstruction-test were easier to run. For example, have a script that downloads and suitably formats the appropriate data and then runs the test. Right now there is some hacky stuff going on with a temp dir that probably isn't in the source tarball.

Ok I think a better way is that the file itself should be a runnable file with passing arguments; something like

`./reconstruction-test --dataset-dir --sample-rate

arguments passed by should be

  • --dataset-dir: An address to a dataset directory which includes some .wav files.
  • --sample-rate: The sample rate in which the comparison is supposed to work with.
  • --release-df: If this flag is appeared, then the procedure will create a set of pandas dataframes saved in .csv format which could be used with pandas later.
  • --release-log It prints out a tab delimited log file which will show the output of comparison.

This will shorten the code and also reduces dependancies.

I was only printing out the bits in the file as a debugging step. But I think we should be displaying the file sizes or bits per unit time in some form alongside the various compression methods; this will make comparisons more meaningful.

The output of the file can be in following shape

Results on sampling rate = <sampling rate>
<Filename>    <algorithm>   <bitrate>   <psnr> [repeating for each compression algorithm]

I just set the bits per sample to 6 in reconstruction-test.py

By that you mean 16? Sorry here I just got confused.

but we may want to test various settings. What I would really like is some info about PSNR's and file sizes formatted in a way that's easily readable.

Did I cover that in previous part?

IMO a simple Python program that prints out a summary would be preferable to this CSV stuff, in that it would be clearer. But as long as you do print out something readable at some point (maybe as a post processing step that might be in a separate script) it's OK with me. Make sure it's all documented with a top-level run.sh or something.

Ok I think a top level .sh with man documentation is what you want here. Right?

danpovey commented

`./reconstruction-test --dataset-dir --sample-rate

Good idea.. but be sure to have a top-level run.sh that runs the complete test.

arguments passed by should be

  • --dataset-dir: An address to a dataset directory which includes some .wav files.
  • --sample-rate: The sample rate in which the comparison is supposed to work with.
  • --release-df: If this flag is appeared, then the procedure will create a set of pandas dataframes saved in .csv format which could be used with pandas later.
  • --release-log It prints out a tab delimited log file which will show the output of comparison.

Sure.

I just set the bits per sample to 6 in reconstruction-test.py

By that you mean 16? Sorry here I just got confused.

No I meant 6- look at the code. The bits-per-sample used by lilcom to compress.

but we may want to test various settings. What I would really like is some info about PSNR's and file sizes formatted in a way that's easily readable.

Did I cover that in previous part?
Yes, I think so.

IMO a simple Python program that prints out a summary would be preferable to this CSV stuff, in that it would be clearer. But as long as you do print out something readable at some point (maybe as a post processing step that might be in a separate script) it's OK with me. Make sure it's all documented with a top-level run.sh or something.

Ok I think a top level .sh with man documentation is what you want here. Right?

Yes, a top-level sh. Just put comments in where needed. man documentation is super overkill for a test script.

soroushzargar commented

`./reconstruction-test --dataset-dir --sample-rate

Good idea.. but be sure to have a top-level run.sh that runs the complete test.

We can make the python as a runnable shell file just with ! /bin/python or sth, but I think you want sth more than just adding this and saving as a sh file. Can you say what you expect from the sh file which this additional line can not do?

I just set the bits per sample to 6 in reconstruction-test.py

By that you mean 16? Sorry here I just got confused.
No I meant 6- look at the code. The bits-per-sample used by lilcom to compress.

Yes found it. we can add another passing parameter to let the user change this value. Is it Ok or sth else is needed.

Ok I think a top level .sh with man documentation is what you want here. Right?

Yes, a top-level sh. Just put comments in where needed. man documentation is super overkill for a test script.

The argument parser gives a help function in which the user can easily find out how to work with the test.

danpovey commented

The idea of run.sh is that it should just run automatically when you run
./run.sh
so the user does not have to spend time figuring out documentation.

soroushzargar commented
soroushzargar commented

In addition to the assumption that MP3 will compress with the exact bitrate passed also there would be an issue with sample rate. Let me test it on long audio too and I'll return the code with true bitrates for mp3.

Do you think we should substitute pydub and librosa with sox. If it is necessary I try to figure it out.

danpovey commented
danpovey commented