Søren Hauberg wrote:
> 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.
It's for the sake of "free toolboxes for matlab". The majority of
OpenOffice users are using it on top of windows. So why not using free
octave toolboxes on top of M ? And it does neither hurt linux nor octave
if they do so.
>
> 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.
done
>
>> --- 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.
ok.
>
>> --- 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.
Are You saying that the function de_min() is broken and nobody cares
about fixing it? I checked and found that nth() is used 44 times in 22
functions (in main/optim, main/vrml and extra/tk_octave) using the
function nth().
Shall I check in the fixes for using nth()?
>
> 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.
Yes, it can be done. Actually, I changed it in such a way that
parenthesis are always used for the RHS.
However, this comes with the cost that it breaks this test case:
y += [1; a](:); ## not correctly resolved
is converted to
o2m_tmp_7 = [1); a];
y = y + (o2m_tmp_7(:); %# not correctly resolved
>
>> --- 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?
The different numbers of left and right parenthesis was noted by
oct2mat, and I could not resist pointing this out.
>
> Søren
>
Alois
------------------------------------------------------------------------------
_______________________________________________
Octave-dev mailing list
Octave-dev@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/octave-dev
