search

ivapylibs / puzzle_solver

1 stars 2 forks source link

Code: puzzle piece template #1

Closed pv33 closed 2 years ago

pv33 commented 3 years ago

Part python and part Matlab coding of puzzle.piece.template is provided. Most parts are documented, but not all.

Figure out who can get to first and take ownership of issue.

pv33 commented 3 years ago

Make sure to update Project kanban status when taking ownership and working on. Automation will automatically move to done when closed.

Uio96 commented 3 years ago

Sure. I will work on them.

Last two days, I spent more time thinking about the design of the puzzle solver and posted my thoughts on the Trello card. For the rest of the week, I will mainly work on the code stuff.

pv33 commented 3 years ago

@Uio96 Puzzle piece should not have an id nor an id_sol. Even if it had the id, it should not have an id_sol. Please remove.

With high likelihood, if it is missing a member variable, then it doesn't need it. I am not sure why an id was added.

Could you please justify why such a variable is needed? If cannot, then please remove.

Uio96 commented 3 years ago

@Uio96 Puzzle piece should not have an id nor an id_sol. Even if it had the id, it should not have an id_sol. Please remove.

With high likelihood, if it is missing a member variable, then it doesn't need it. I am not sure why an id was added.

Could you please justify why such a variable is needed? If cannot, then please remove.

I suggest using an id, which is a better and flexible option than checking its index in the board instance. If some pieces are deleted or missing, the index in the pieces of the board will change. So if we want to do some complicated processing in the future, we can easily track its id without any trouble. It is a common practice in the tracking works I have been working around.

I agree with you that id_sol may not be necessary. It can be managed by pAssignments in the manager instead.

pv33 commented 3 years ago

@Uio96 The indexing into the solution board provides the ID. The puzzle.simple solver consists of a data association list that maps to the solution board. It keeps track of the association. That's what self.match is, which it looks like you have chosen to not implement without discussing. That goes against the grain of collaborative coding. Such a major structural change should have triggered a git issue discussion thread.

As designed in the code stubs and based on the layered approach, there will be multiple layers of boards that keeps track of things. Deleted or missing pieces will trigger a revision of the data association. Deletion should be invoked at the solver level and not at the board level. It then applies to the board and also takes care of own house keeping.

If an ID turns out to be needed, then it might actually be better to do so for a sub-class. Definitely there should not be two IDs.

The counter argument is that the piece can self-manage the identifier. Not much extra work is needed when taking care of the puzzle piece, e.g, during addition and deletion.

Currently, the puzzle solving problem is similar to, but not exactly the same as tracking in a surveillance system. Tracking has many unknown objects coming in and out, and there must exist a mechanism to handle them. It makes sense to give each detection an ID because it may or may not be associated. A puzzle has a fixed set of known objects. Every puzzle board must have an association to a known piece. If implementing, then there should ever only be one ID ever and not two IDs.

The ID can be unknown (maybe -1) until matched, but should not ever permit there being more IDs than puzzle pieces (+1 for unmatched).

At least remove id_sol because it is frivolous. At most, both will need to go.

Right now my preference is for there to be a data association member variable. That way, board-level operations can be done in a vectorized manner. It can stay as is for now, but at some point it might get reverted to the original coding. We'll see if vectorization is possible using this technique. Just be ready in case it doesn't.

Just FYI:

By a layered approach, what will happen is that there will most likely be 6 puzzle board layers.

  1. Solution puzzle
  2. Estimated puzzle (all pieces and estimated locations)
  3. Measured/Visible puzzle (all visible pieces)
  4. Occluded pieces (obscured by hand and robot)
  5. Picked by hand puzzle piece (single piece identified as picked up)
  6. Picked by robot puzzle piece (single piece identified as picked up)

The last two will still be board instances for extensibility to the two-handed case (worker or robot).

It might make sense to let the pieces store the ID. The list then has to be pulled out for vectorization operations. Can a list of classes be queried for a member variable to return a list of that variables values? Something like:

a(:).field

In Matlab, that would return an array of values from field. If so, then vectorization will involve first getting the list, then applying it to reshuffle the pieces. When done as originally programmed, the list calculation is avoided, but then list management is needed. I'll think about it over the weekend.

Uio96 commented 3 years ago

@Uio96 The indexing into the solution board provides the ID. The puzzle.simple solver consists of a data association list that maps to the solution board. It keeps track of the association. That's what self.match is, which it looks like you have chosen to not implement without discussing. That goes against the grain of collaborative coding. Such a major structural change should have triggered a git issue discussion thread.

As designed in the code stubs and based on the layered approach, there will be multiple layers of boards that keeps track of things. Deleted or missing pieces will trigger a revision of the data association. Deletion should be invoked at the solver level and not at the board level. It then applies to the board and also takes care of own house keeping.

If an ID turns out to be needed, then it might actually be better to do so for a sub-class. Definitely there should not be two IDs.

The counter argument is that the piece can self-manage the identifier. Not much extra work is needed when taking care of the puzzle piece, e.g, during addition and deletion.

Currently, the puzzle solving problem is similar to, but not exactly the same as tracking in a surveillance system. Tracking has many unknown objects coming in and out, and there must exist a mechanism to handle them. It makes sense to give each detection an ID because it may or may not be associated. A puzzle has a fixed set of known objects. Every puzzle board must have an association to a known piece. If implementing, then there should ever only be one ID ever and not two IDs.

The ID can be unknown (maybe -1) until matched, but should not ever permit there being more IDs than puzzle pieces (+1 for unmatched).

At least remove id_sol because it is frivolous. At most, both will need to go.

Right now my preference is for there to be a data association member variable. That way, board-level operations can be done in a vectorized manner. It can stay as is for now, but at some point it might get reverted to the original coding. We'll see if vectorization is possible using this technique. Just be ready in case it doesn't.

Just FYI:

By a layered approach, what will happen is that there will most likely be 6 puzzle board layers.

  1. Solution puzzle
  2. Estimated puzzle (all pieces and estimated locations)
  3. Measured/Visible puzzle (all visible pieces)
  4. Occluded pieces (obscured by hand and robot)
  5. Picked by hand puzzle piece (single piece identified as picked up)
  6. Picked by robot puzzle piece (single piece identified as picked up)

The last two will still be board instances for extensibility to the two-handed case (worker or robot).

It might make sense to let the pieces store the ID. The list then has to be pulled out for vectorization operations. Can a list of classes be queried for a member variable to return a list of that variables values? Something like:

a(:).field

In Matlab, that would return an array of values from field. If so, then vectorization will involve first getting the list, then applying it to reshuffle the pieces. When done as originally programmed, the list calculation is avoided, but then list management is needed. I'll think about it over the weekend.

Thank you for the clarifications.

I added the id stuff for puzzle.piece.template before you created the puzzle.solver. That's the reason why there might be some overlapping. Next time, I will create a GitHub issue if I want to make some structural changes.

Right now, I will keep the id for each puzzle piece instance but remove id_sol.