interpolat

finds brackets of x that contain sign changes of a function on the interval [ xmin , xmax ] and uses

Description

finds brackets of x that contain sign changes of a function on the interval [ xmin , xmax ] and uses interpolation method to find the root of a function Example: fp=inline ( '(-12)+(-21.*x)+(8*x.^2)-(2.75*x.^3)','x');
A=interpolat(fp,2,-1,1000,3,0.001,100)
A = -0.47438852171570

• MATLAB release: MATLAB 7.1.0 (R14SP3)

[John D'Errico]

My thanks to Richard Brown for his accurate review. Most of my comments would just mirror his. This submission indeed needs very much improvement.

The help is poor, consistently so for this author. (I'm disappointed that this author has not improved his coding style given the many poor reviews.) While there are short descriptions of the parameters, they are woefully inadequate. For example, es is described as a stopping criterion. But what test is applied, we don't have any idea. maxit is the maximum number of iterations, but what is iterating? How many iterations is a reasonable number?

Worse, while the many parameters have default values, we don't have any idea what those defaults are. Worse yet, these defaults are poorly implemented. If you wish to set only the last variable to something other than its default value, you must set all the other variables too. And, since we don't know what the defaults for the other variables are, this is a real problem. This goes against the standard matlab paradigm for defaults.

I also agree with Richard's comments about the methods used, and am not at all impressed with the Gauss-Seidel method chosen. The algorithm itself is a bit suspect, not converging to any requested tolerances in my tests.

Poor programming styles are used throughout. For example, a Vandemonde matrix is generated using a doubly nested loop. Why not use the existing vander function? Even better, since this was created using the current release, use bsxfun to generate that matrix in one line, with NO loops at all.

Other loops are used for no good reason, where a simple vectorized statement is entirely appropriate.

The code is not terribly readable, lacking whitespace. While it does have many internal comments (a GOOD thing!), they are not always useful. Many comments merely name the loop variable, something that is usually clear anyway. Far more valuable would be a useful explanation of the method used, something that is totally lacking.

Other examples of poor coding style are the use of poor variable names. Use of a lower case "L" is a bad idea, as it makes your code virtually impossible to read. l looks too much like the number 1.

Even the name of the function is a poor, un-mnemonic choice. Why call a root finding code "interpolat"? If the next homework assignment by this author is truly an interpolation code, I might expect to see it called rootfind.m?

Next, this function lacks an H1 line. Its a necessary tool to enable the lookfor command. And since the name of this function is so un-memorable, one would think lookfor is a valuable thing.

Finally, mlint flags fully 15 different lines in this code. Many of those flags are problems worthwhile resolving.

As I said, this code needs vast improvement before I would even consider calling it fair. This is not to say the code is terrible, as I've seen far worse submissions on the FEX. While part of me wanted to rate this as "poor", it has barely enough redeeming characteristics to call it "needs improvement". But there are other codes I would recommend before this one.

[Richard Brown]

I started writing, and then it started turning into an epic, so I'll summarise
* Hard to see the point in using this - fzero plus some method of estimating root locations would be a more obvious choice
* Difficult to read - whitespace, choice of variable names
* No vectorisation - lots of nested loops
* Input parameter passing is flawed - try parameter/value pairs - see parse_pv_pairs by John D'Errico for instance
* Bad method of interpolation - uses Gauss Seidel (a strange looking implementation written by Steven Chapra) to solve a Vandermonde matrix equation. Gauss Seidel is not a good choice of solver here (the system is not diagonally dominant, and thus Gauss Seidel is not guaranteed to converge), backslash is a much better choice. The Vandermonde matrix is created in reverse using 2 nested loops, and then flipped. This can be done in one line using vander. What's more, the comment on the line where the coefficients are solved for informs me that Gauss Seidel is computing the eigenvalues of a matrix. Try using Newton's divided difference method or similar for interpolating.

On a positive note, there is error checking and quite a bit of internal commenting.

The idea, I think has some merit, but a substantial rewrite is recommended with clear documentation, sensible choice of techniques, and vectorisation.

Don't use that piece of Gauss Seidel code for a model of well-written Matlab code. See some of the submissions by some of the top FEXers for examples

[Jos x@y.z]

So, why shouldn't we use matlab's own FZERO ...