[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Ok, new problem
Henry F. Mollet
Re: Ok, new problem
Mon, 19 Nov 2007 18:24:36 -0800
on 11/19/07 11:07 AM, Przemek Klosowski at address@hidden wrote:
> I probably sound like an old grumpy man saying this, but you should
> rethink the style of your programming. You tend to use a single large
> subroutine to do the entire task, which makes it hard to find and
> debug problems---you can't see what's happening inside your code.
> This is actually not even specific to Octave---having worked with
> scientific programming for a long time, I see a lot of Fortran, C,
> Octave and whatever else, written in this 'On The Road' streaming
> You need to break it up into small, agile, testable little
> pieces. This is good for several reasons, some actually relevant to
> the questions you posted:
> - it's easier to see logical and syntax error in smaller modules
> - it's easier to test small modules that return intermediate values
> that you can examine for correctness
> - you exploit patterns and avoid cut-and-paste repetition errors
> - you can better see relationships and commonalities between modules.
> - it's easier to evolve code
> For instance, note that you probably made a mistake for the third
> term: you have f3=f*6, where I think you meant f*3, but it's hard to
> see when you write it like you did.. Also, my version of octave
> requires the following usage of wavwrite():
> wavwrite (filename, y, samples_per_sec, bits_per_sample)
> the ordering of parameters you used in your program doesn't work for me.
> People did studies on programming ergonomics, and came up with a
> rule of thumb that code modules should have 12 lines or less. Of
> course some of those lines may be macros, or subroutine calls, so
> this doesn't limit the complexity of your code---but you have to
> give some thought to how you split it up.
> I would rewrite your code thus:
> function w=oneChirp(t,tau,A,f)
> function w=sinte(f,Fs,T)
> t = 0:1/Fs:T;
> w = oneChirp(t,.044,28184, f) + \
> oneChirp(t,.036, 7943.3, 2*f) + \
> oneChirp(t,.048, 2818.4, 6*f) + \
> oneChirp(t,.015, 7943.3, 4*f) + \
> oneChirp(t,.021, 1778.3, 5*f) + \
> oneChirp(t,.018, 2818.4, 6*f) + \
> oneChirp(t,.016, 1258.9, 7*f) + \
> oneChirp(t,.017, 794.3, 8*f) + \
> oneChirp(t,.038, 100.0, 9*f) + \
> oneChirp(t,.017, 562.3,10*f) ;
> Fs = 44100;
> f=300 ;
> Note that you can plot and see the resulting waveform and even plot
> component parts:
> plot(t, w)
> so you exploit the scriptable, immediate execution nature
> of Octave for ad-hoc debugging and manipulation.
> Your parameters A and tau---if you didn't just pull them out of your
> hat, you probably would calculate them, and they would come out of
> an array. It'd be easy to rewrite the code above to use it:
> for i=1:10
> w += oneChirp(t, tau(i), A(i), f*i)
> or even vectorize by changing oneChirp() to take array values as
> parameters, and return the sum of terms directly.
> There are some disadvantages to very modular style. You tend to use
> more function parameters (note my use of Fs and T in the sinte()
> call)---the only alternative being global variables, which would
> not be recommended. Also, too many modules and function calls can be slow,
> but that is not a concern here.
Thanks for providing guidelines for programming.
I have 2 questions.
1. I guess that A has to be provided because:
warning: function name `oneChirp' does not agree with function file name
error: `A' undefined near line 2 column 10
error: evaluating binary operator `*' near line 2, column 11
error: evaluating binary operator `.*' near line 2, column 23
error: evaluating assignment expression near line 2, column 9
error: called from `tempOctaveWavFile2' in file
2. wavwrite and wavread have different syntax:
octave-2.9.17:3> help wavwrite
-- Function File: wavwrite (Y, FILENAME)
-- Function File: wavwrite (Y, FS, FILENAME)
-- Function File: wavwrite (Y, FS, BITS, FILENAME)
Write Y to the canonical RIFF/WAVE sound file FILENAME with sample
rate FS and bits per sample BITS. The default sample rate is 8000
Hz with 16-bits per sample. Each column of the data represents a
See also: wavread.
octave-2.9.17:2> help wavread
-- Function File: Y = wavread (FILENAME)
Load the RIFF/WAVE sound file FILENAME, and return the samples in
vector Y. If the file contains multichannel data, then Y is a
matrix with the channels represented as columns.
-- Function File: [Y, FS, BITS] = wavread (FILENAME)
Additionally return the sample rate (FS) in Hz and the number of
bits per sample (BITS).
-- Function File: [...] = wavread (FILENAME, N)
Read only the first N samples from each channel.
-- Function File: [...] = wavread (FILENAME,[N1 N2])
Read only samples N1 through N2 from each channel.
-- Function File: [SAMPLES, CHANNELS] = wavread (FILENAME, "size")
Return the number of samples (N) and channels (CH) instead of the
See also: wavwrite.