RE: 5x5 Arithmetic solver

[Vincent Belaïche]

Could you please send me the diff compared to my contribution, rather than compared to the official latest 5x5, that would help me understand your suggestion.

VBR,
   Vincent.

> Date: Sat, 21 May 2011 11:36:52 -0300
> From: address@hidden
> To: address@hidden
> Subject: Re: 5x5 Arithmetic solver
> CC: address@hidden; address@hidden
>
> Hi Vincent,
>
> Could I suggest the attached diff?
>
> Vinicius
>
>
> On 05/21/2011 04:15 AM, Vincent Belaïche wrote:
> > Salut Stéfan,
> >
> > Toujours aussi à cheval sur les conventions de codage !
> >
> >>> Please find herein attached a contribution to the 5x5 game. This is an
> >>> arithmetic solver based on a matrix inversion in a (Z/2Z)^25 vector
> >>> space.
> >> Thanks. A few comments:
> >> - avoid using a tarball and just attach the diff as-is,
> >> makes it a lot easier to review.
> > noted
> >
> >> - why 5x5-local-variables?
> > That is used in 5x5-mode to imply that all listed variable are made
> > local to that very 5x5 buffer. if after `M-x 5x5' you do a `M-x
> > rename-uniquely' and then `M-x 5x5' again then you have have two
> > independent games.
> >
> >> - explain the changes in the 5x5 function.
> > I had to change slightly the order of operations because setting the
> > mode has to be done before any buffer local 5x5 variable is touched, as
> > precisely those variables are made local by setting the mode
> >
> >> - many of your lines have trailing whitespace. I generally don't care
> >> much, about it, but some people do, and it's usually preferable to
> >> avoid them. M-x picture-mode C-c C-c gets rid of them for you (as
> >> a side-effect).
> > Done
> >
> >> - try to keep the first line of docstrings as a self-sufficient sentence
> >> (because M-x apropos only shows the first line).
> >
> >
> >> - stay within 80 columns.
> > Do you mean that we are still in the 80ies ;-P ?
> >
> > Ok, done.
> >
> >> - your code is not properly indented (e.g. the `grid' argument in
> >> 5x5-grid-to-vec).
> > Done
> >
> >> - Please capitalize your comments and terminate them with a "." or some
> >> other appropriate punctuation.
> > Some comments by a variable name, so they are not capitalized.
> >
> > Some comments are equations like this:
> >
> > ;; B:= targetv
> > ;; A:= transferm
> > ;; P:= base-change
> > ;; P^-1 := inv-base-change
> > ;; X := solution
> >
> > ;; B = A * X
> > ;; P^-1 * B = P^-1 * A * P * P^-1 * X
> > ;; CX = P^-1 * X
> > ;; CA = P^-1 * A * P
> > ;; CB = P^-1 * B
> > ;; CB = CA * CX
> > ;; CX = CA^-1 * CB
> > ;; X = P * CX
> >
> > I don't think that this is common practice to use a punctuation at the
> > end of an equation.
> >
> > Some other comment is commented out code like this:
> >
> > ;;(ctransferm-1-2 (calcFunc-mcol ctransferm-1-: col-2))
> >
> > This code is not there because I don't need that variable, but I found
> > useful to show how it would be defined.
> >
> > For all the remaining comments I have done it
> >
> >> - 5x5-solve-suggest should have a docstring.
> > Done
> >
> >> - try C-u checkdoc-current-buffer.
> > I get
> >
> > checkdoc-continue: Too many occurrences of \[function]. Use \{keymap}
> > instead
> >
> > Because 5x5 is used many times as if it was
> >
> >
> >> - we need a ChangeLog entry.
> > Here it is:
> >
> > -----------------------------------------------------------------------
> > 2011-05-21 Vincent Belaïche<address@hidden>
> >
> > * play/5x5.el: Add an arithmetic solver to suggest positions to
> > click on.
> > -----------------------------------------------------------------------
> >
> >> - I don't understand the "solve step" message (e.g. it said 23 every
> >> time, even though I followed its suggestions and finished in 12 moves).
> >>
> >>
> > Ask it to Jay, this message is output by Calc, not by 5x5. 23 is due
> > to this that you have to invert a 23x23 matrix. Altough the 5x5 transfer
> > matrix is 25x25, its rank is only 23, so I extract some submatrix to
> > compute the solution.
> >
> >> Stefan
> > Vincent.
> >
>