3

I am trying to solve the n-queens problem. I wrote a solution for the problem as follows:

def isSafe(board,row,col):
    for i in range(row):
        if board[i][col]==1:
            return False
        
    x,y = row,col
    # print(x,y)
    while x>=0 and y>=0:
        if board[x][y]==1:
            return False
        x-=1
        y-=1

    x,y = row,col
    # print(x,y)
    while x>=0 and y<len(board):
        if board[x][y]==1:
            return False
        x-=1
        y+=1

    return True
        
        

def solveNQueens(n):
    ans = []
    def placeQueens(board,row):
        if row==len(board):
            ans.append(board)
            return

        for i in range(len(board)):
            if isSafe(board,row,i):
                board[row][i]=1
                # print(board)
                placeQueens(board,row+1)
                board[row][i]=0
        return
                
        
    
    board = [[0 for i in range(n)] for j in range(n)]
    # print(board)
    
    placeQueens(board,0)
    
    print(ans) 

solveNQueens(4)

The above code uses recursive backtracking to solve the n queens problem. The placeQueens function appends the board combination directly to ans list when all the queens are assigned a row but when I try to print ans, all the places are marked as zeroes.

e.g for n=4:

Expected ans = [[[0,1,0,0],[0,0,0,1],[1,0,0,0],[0,0,1,0]],[[0,0,1,0],[1,0,0,0],[0,0,0,1],[0,1,0,0]]]

Answer printed = [[[0,0,0,0],[0,0,0,0],[0,0,0,0],[0,0,0,0]],[[0,0,0,0],[0,0,0,0],[0,0,0,0],[0,0,0,0]]]

Where am I going wrong??

I tried appending the board positions one by one to the ans list and the code works fine.

def isSafe(board,row,col):
    for i in range(row):
        if board[i][col]==1:
            return False
        
    x,y = row,col
    # print(x,y)
    while x>=0 and y>=0:
        if board[x][y]==1:
            return False
        x-=1
        y-=1

    x,y = row,col
    # print(x,y)
    while x>=0 and y<len(board):
        if board[x][y]==1:
            return False
        x-=1
        y+=1

    return True
        
        

def solveNQueens(n):
    ans = []
    def placeQueens(board,row):
        if row==len(board):
            ans.append([])
            # print(board)
            for i in range(len(board)):
                ans[-1].append([])
                for j in board[i]:
                    if j==0:
                        ans[-1][-1].append(0)
                    else:
                        ans[-1][-1].append(1)
            return

        for i in range(len(board)):
            if isSafe(board,row,i):
                board[row][i]=1
                # print(board)
                placeQueens(board,row+1)
                board[row][i]=0
        return
                
        
    
    board = [[0 for i in range(n)] for j in range(n)]
    # print(board)
    
    placeQueens(board,0)
    
    print(ans)

solveNQueens(4)

Why is the first code generating incorrect output, when both the codes are logically similar??

Will Ness
  • 70,110
  • 9
  • 98
  • 181
Sanjit Jha
  • 33
  • 4
  • Neither of these are valid python code. What's with the double asterisks? – jprebys Jan 04 '23 at 20:20
  • Also, what is the board object? Is it from the python chess library? – Nonlinear Jan 04 '23 at 20:22
  • Please post the full code, so we can run it ourselves. Presumably these functions are members of a class, but you have not shown us the class definition, nor how the class object is instantiated/called. – John Gordon Jan 04 '23 at 20:22
  • Sorry for the error. I have edited the code. Would be very helpful if you could review the code and provide the necessary assistance – Sanjit Jha Jan 04 '23 at 20:22
  • @chrslg I am very sorry for the mess. This was my first time posting a question.I would avoid repeating it. Thanks for your guidance. – Sanjit Jha Jan 04 '23 at 20:33
  • ```board[row][i]=1``` is shortly zeroed out by ```board[row][i]=0```, what are you intending with these two lines. – jwal Jan 04 '23 at 20:40
  • @jwal : that is not the problem. It is a recursive function. `board[row][i]` needed to be changed just the time to recursively try with this hypothesis, and then unchanged to try the next hypothesis. This is how this kind of recursive search work. – chrslg Jan 04 '23 at 22:05
  • @chrslg locally scoped variables; ```row``` and ```i```; are probably not behaving the way you are assuming. – jwal Jan 05 '23 at 00:22
  • Interesting to look at other [solutions](https://www.geeksforgeeks.org/python-program-for-n-queen-problem-backtracking-3/) which exit before setting the board value back to zero. – jwal Jan 05 '23 at 00:47
  • @jwal I assure you they do. I've taught this my whole life: this is very classical branch&bound; `row` and `i` are doing exactly what we usually do in b&b. It is locally scoped `board` that does not behave as OP intended (because `board` is local, but its content is not). – chrslg Jan 05 '23 at 01:22
  • 1
    @jwal setting a cell back to 0 after the recursive call is the essence of the recursive backtracking technique used here. this code collects all solutions so can not be exiting after the first one is found, but even if it were to do so it would still have to set the cell to 0 after that recursive call because a first valid queen placement in a certain row might still lead to a dead end further down, so the next position must be tried. – Will Ness Jan 06 '23 at 01:52
  • 1
    Now that you've got your answer, here's a thought: 0 cell is safe. Place a queen on a 0 cell and mark it *and* all the cells it threatens by incrementing the cell's contents. – Will Ness Jan 06 '23 at 06:37

1 Answers1

2

So, you have, correctly, avoided the trap that arrays are "pointers" (see note at last paragraph), and therefore that using several time the same array would make all the instance alias of each other here:

board = [[0 for i in range(n)] for j in range(n)]

You were right to do so. The trap here would have been to say

board = [[0]*n]*n

Note that the inner [0]*n would not have been a problem. But the outer one, would have lead to the well known situation where changing board[1][2]=1 for example, would have changed board to [[0, 0, 1, 0], [0, 0, 1, 0], [0, 0, 1, 0], [0, 0, 1, 0]], because all board[i] are the same array of ints.

Reason why I take time to explain you something you apparently already know, and to explain a mistake you haven't made, is that this is the same reason why your code doesn't work. Because "arrays are pointers".

So, here

        if row==len(board):
            ans.append(board)

what you append here is board. An array. So a "pointer". If board content change after that, it will impact the content of what you have added to ans. So the content of ans. Each thing you appended to ans, are just several time the same board (so, even if it were not full 0, you were doomed to have only identical boards in ans). And even more, since at the end of your code, board is full 0, well ans if several times board, that is several time full 0.

Just, replace

ans.append(board)

by

ans.append([[board[i][j] for j in range(n)] for i in range(n)])

(or ans.append([x.copy() for x in board]), but the double loop has the advantage to resemble what you did for initialization)

So to add to ans not board but a new array whose content is the same as board content (and that will not change when board changes), and the result is (I've just tried, with your first code, and only this change)

[[[0, 1, 0, 0], [0, 0, 0, 1], [1, 0, 0, 0], [0, 0, 1, 0]], [[0, 0, 1, 0], [1, 0, 0, 0], [0, 0, 0, 1], [0, 1, 0, 0]]]

That is exactly what you were expecting.

So, your code (the recursive search, and all) works exactly as intended. Your only problem was a "pointer" problem

Note: when I say "pointer" it is by analogy with some other languages were you can use "pointer" to creates some sort of alias of a variable. In reality it is not accurate to say thing like "arrays are pointers". It is not like not all data type behave the same in python from this point of view. There aren't some data-type that are aliased and other that are copied. It is just that you wouldn't have made this mistake with, for example a string, simply because string being unmutable, if board had been a string, it couldn't have changed.

I ranted a while on this matter in another post

chrslg
  • 9,023
  • 5
  • 17
  • 31
  • This is not an elegant solution. You add complexity to a problem that has a nicer solution and spend too much time ranting about something not relevant to the OP. – jwal Jan 05 '23 at 00:48
  • 1
    @jwal But that is not the point. The point is to answer the question, not to solve n-queen problem. Again, I've taught this my whole life. I know many other solution, including better one, to n-queens problem. But the objective of SO is not to solve classical problem, but to answer coding question. Coding question was "why it doesn't work". And reason why this code, as inelegant you may find it, doesn't work, relies only in the fact that `board` content was not copied to ans, and therefore `ans` content change after it is added. – chrslg Jan 05 '23 at 01:24
  • 1
    @jwal The whole point of my answer is to be as much as possible the exact copy of OP code, but for that bug correction. Not to taught OP how else to solve n-queens, which they didn't ask for. The point of the code I provide is not to provide code, but to prove my point on what the bug is. That was the question: where is the error. That should any way always be the question on SO: not "this is my task, I want you to do it for me", but "this is my code, it doesn't work as intended, and I would like to understand why", as the OP asked. – chrslg Jan 05 '23 at 01:26
  • 1
    @jwal Now, I feel a bit of "internet fight" in your remark (especially the first), a bit of revenge for having told that your suspected bug (the double assignment in short period of `board[row][i]`) wasn't the right problem (there was nothing personal. Just, it was really not the problem, and I don't feel it should hurt anybody that I tell so). I won't participate more in that. Feel free to downvote my answer and provide a better one. – chrslg Jan 05 '23 at 01:33
  • 1
    Deepcopy is likely a better approach, and the intent is certainly clearer. – MatBailie Jan 05 '23 at 11:16
  • 1
    @MatBailie Yep. As I said in comment, I used the "compound copy" because OP has used a similar code in their code. And I didn't import `copy` because I wanted the code change to be minimal, to pinpoint the bug. – chrslg Jan 05 '23 at 11:26
  • 1
    @MatBailie Instantiating a new `board` would be the best idea if we could continue with a `0` board. But because of recursive search, we need, even after we found a solution and added it to `ans`, to keep `board` intact (`board` content, that is): further solution may be found backtracking from this one. So that new `board` should be initialized with the same values as the old one. In other words, we still have the copy to do. It do. And whether it is the one that is in `ans` that is the copy and the one in `board` variable the original, or the other way round, really doesn't matter. – chrslg Jan 05 '23 at 11:27
  • Yours certainly is correct and right answer. I've left a comment under the question which suggests a way that makes copying the board as we go down the recursion chain advantageous, I think. – Will Ness Jan 06 '23 at 06:45