tor, 13 05 2010 kl. 23:09 +0200, skrev Alois Schlögl:
> Oct2mat has significantly improved. Most notable, support for the
> following language elements as been added:
>
> - support for standalone ++, --,
> - support of +=, -= etc operators,
> - support of [blah](ix), fun(a)(ix)
> - do...until,
> - multiple assignments like a=b=c=d
> - variable names starting with "_" (underscore)
>
> This means, that many more functions can be converted without manual
> editing. The patch that corrects known issues in about 100 files have
> reduced to about 22 files (from over 3000 lines to less than 700 lines).
> This revised patch is attached.
>
> Some comments on the patch:
> ===========================
> (i) n(k)+=1; is preferred over n(k)++ for two reasons
> ------------------------------------------------
> 1) the former is faster
> octave:36> N=1e6;
> octave:37> tic;n=0;for k=1:N, n++;end;toc;
> Elapsed time is 0.48 seconds.
> octave:38> tic;n=0;for k=1:N, n+=1;end;toc;
> Elapsed time is 0.26 seconds.
Odd. This looks like something that should be improved in Octave. Would
you care to file a bug?
> 2) for indexed variables, n(k)+=1 is correctly converted by oct2mat
> while conversion of n(k)++ is not supported.
>
> (ii) a+=b|c, a-=b+c etc. would be incorrectly resolved to a = a+b|c, and
> a = a-b+c. For this reason, it is sometimes necessary to add paranthesis
> a+=(b|c). [Remark: there is a solution to introduce always (RHS), but
> this breaks another example. You can test oct2mat with this commands:
> mkdir /tmp/oct2mat
> cd ~/octave-forge/extra/oct2mat/inst
> ./oct2mat test_oct2mat.m
> cat /tmp/oct2mat/test_oct2mat.m
>
> 3) increment ++ and decrement -- operators within another expression can
> not be correctly converted. Therefore, manual editing is necessary.
Sounds to me like something that should be fixed in 'oct2mat'.
> As far as I understand the various comments, only Olaf is against the
> patch.
Personally, I am not against the patch, but I wouldn't say that I am for
the patch either. I am not a fan of rewriting perfectly functional code
in order to help an automated Matlab converter.
As to the suggested patches, then I few things caught my attention:
> --- main/image/devel/__conditional_mark_patterns_lut_fun__.m (revision 7295)
> +++ main/image/devel/__conditional_mark_patterns_lut_fun__.m (working copy)
I will not accept these changes to this function as this is not a
function the user should call, but rather a function that used to
generate code in the 'image' package. This function, thus, more has a
value as documentation to developers rather than to users.
> --- main/image/inst/imhist.m (revision 7295)
> +++ main/image/inst/imhist.m (working copy)
> @@ -62,8 +62,11 @@
>
> if (nargout == 2)
> [nn,xx] = hist (I(:), bins);
> - vr_val_cnt = 1; varargout{vr_val_cnt++} = nn;
> - varargout{vr_val_cnt++} = xx;
> + vr_val_cnt = 1;
> + varargout{vr_val_cnt} = nn;
> + vr_val_cnt += 1;
> + varargout{vr_val_cnt} = xx;
> + vr_val_cnt += 1;
I will not accept these changes as I feel it makes the code harder to
read. It also seems to me like it would be better to fix 'oct2mat' to
handle this coding style.
> --- main/image/inst/imsmooth.m (revision 7295)
> +++ main/image/inst/imsmooth.m (working copy)
> @@ -479,7 +479,8 @@
> %for i = 1:10
> i = 0;
> while (true)
> - disp(++i)
> + i+=1;
> + disp(i);
> ms(ms) = ms;
> #new_ms = ms(ms);
> if (i >200), break; endif
This piece of code is part of an algorithm that is not yet fully
implemented. As such, I don't want changes like this in here. I have now
commented out this part of the code, so I don't think it should cause
you problems any more.
> --- main/optim/inst/de_min.m (revision 7295)
> +++ main/optim/inst/de_min.m (working copy)
> @@ -147,7 +147,8 @@
> if nargin > 2,
> ctl = struct (varargin{:});
> else
> - ctl = nth (varargin, va_arg_cnt++);
> + ctl = nth (varargin, va_arg_cnt);
> + va_arg_cnt += 1;
Two comments:
1. 'oct2mat' really should be improved to deal with this coding style.
2. This has to be rewritten anyway ('nth' is no longer part of
Octave), so I find it somewhat silly to make this change.
Similar comments can be made for other patches.
> --- main/time/inst/thirdwednesday.m (revision 7295)
> +++ main/time/inst/thirdwednesday.m (working copy)
> @@ -49,7 +49,7 @@
> wednesdays = nweekdate (3, 4, year, month);
> dates = datevec (wednesdays);
> ## adjust the year when the date will wrap
> - dates(:,1) += dates (:,2) > 9;
> + dates(:,1) += (dates (:,2) > 9);
Why can't 'oct2mat' insert this parenthesis automatically? It can just
always do it.
> --- main/nnet/inst/__calcjacobian.m (revision 7295)
> +++ main/nnet/inst/__calcjacobian.m (working copy)
> @@ -124,7 +124,7 @@
> ## tildeSx = -dFx(n_x^x)
> ## use mIdentity to calculate the number of targets correctly
> ## for all other layers, use instead:
> - ## tildeSx(-1) = dF1(n_x^(x-1)))(W^x)' * tildeSx;
> + ## tildeSx(-1) = dF1(n_x^(x-1))(W^x)' * tildeSx;
Huh? You need to make changes to the comments?
Søren
------------------------------------------------------------------------------
_______________________________________________
Octave-dev mailing list
Octave-dev@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/octave-dev
