You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I have some suggestions for the implementation of CSP class and some of the methods in csp.py.
several methods should belong to the CSP class (creating a CSP object should allow me to check arc consistency within that CSP!)
the dict assignment should belong to the CSP instead of being an external variable that's passed around. It should start as an empty dict and grow until a solution is found. By making this variable and some of the methods local to CSP, one will be able to check arc consistency (AC3(self)) on self.assignment without passing arguments around. This looks cleaner and more elegant to me. It also simplifies the API for students who might want to adapt the code to other purposes. Besides, I think it should be renamed - maybe to solution_states instead of assignment?
currently, CSP.unassign() does nothing that needs it to belong to CSP. CSP.assign() is similar, except that it increments the local self.nassigns. I think this is another point in favour of creating a local CSP.assignment
I can't understand the format of the state argument to CSP.actions() and other functions around it. I can see that we call dict(state), but this is still unclear. In NQueens we use the result of backtrack() as the value of state in goal_test(). actions() is neither called nor overridden anywhere in the code.
I looked at Implementation: Tree CSP Solver #432, but I'm still not entirely certain why we're creating support_pruning functionality. Even for the biggest example we deal with (USA CSP, 50 variables, 4 values in each domain), starting out with a full CSP.domains doesn't seem prohibitively slow. @MrDupin, what do you think?
the CSP.curr_domains should probably be renamed to something more indicative like CSP.reduced_domains.
Besides, the API is generally a bit confusing. Methods are unintuitively named, like 'mac()' which is supposed to maintain arc consistency, but returns a boolean. no_inference (which takes 5 arguments!) returns True every time!
I would like to restructure the code to make it a) readable, and b) easier to use for more generic purposes.
Besides, chapter 6 on CSPs is a concept-heavy chapter, yet the notebook and implementations spend almost no time on driving these concepts home. I would like to implement the PC-2 algorithm for path consistency (should be straightforward, but I'll wait for suggestions on the API), as well as simple examples to illustrate k-consistency and similar concepts. In general, the key concept behind CSPs, which is to shrink the searchspace, seems to be getting lost in the implementation as it is.
I have some suggestions for the implementation of
CSPclass and some of the methods incsp.py.CSPobject should allow me to check arc consistency within thatCSP!)assignmentshould belong to the CSP instead of being an external variable that's passed around. It should start as an empty dict and grow until a solution is found. By making this variable and some of the methods local toCSP, one will be able to check arc consistency (AC3(self)) onself.assignmentwithout passing arguments around. This looks cleaner and more elegant to me. It also simplifies the API for students who might want to adapt the code to other purposes. Besides, I think it should be renamed - maybe tosolution_statesinstead ofassignment?CSP.unassign()does nothing that needs it to belong toCSP.CSP.assign()is similar, except that it increments the localself.nassigns. I think this is another point in favour of creating a localCSP.assignmentstateargument toCSP.actions()and other functions around it. I can see that we calldict(state), but this is still unclear. In NQueens we use the result ofbacktrack()as the value ofstateingoal_test().actions()is neither called nor overridden anywhere in the code.support_pruningfunctionality. Even for the biggest example we deal with (USACSP, 50 variables, 4 values in each domain), starting out with a fullCSP.domainsdoesn't seem prohibitively slow. @MrDupin, what do you think?CSP.curr_domainsshould probably be renamed to something more indicative likeCSP.reduced_domains.no_inference(which takes 5 arguments!) returnsTrueevery time!I would like to restructure the code to make it a) readable, and b) easier to use for more generic purposes.
Besides, chapter 6 on CSPs is a concept-heavy chapter, yet the notebook and implementations spend almost no time on driving these concepts home. I would like to implement the PC-2 algorithm for path consistency (should be straightforward, but I'll wait for suggestions on the API), as well as simple examples to illustrate k-consistency and similar concepts. In general, the key concept behind CSPs, which is to shrink the searchspace, seems to be getting lost in the implementation as it is.