thewca / tnoodle

Development for the official WCA scramble server
https://www.worldcubeassociation.org/regulations/scrambles/
GNU Affero General Public License v3.0
393 stars 93 forks source link

Threephase Integration cleanups #62

Closed lgarron closed 12 years ago

lgarron commented 12 years ago

Threephase appears to be fast, and not too slow to initialize, which is awesome.

clementgallet commented 12 years ago

I added a peek to the pruning table generation status, so that the init progress is now smooth.

lgarron commented 12 years ago

The smooth progress works well, but it doesn't seem to be integrated with the rest of the progress. If I add one round of 4x4x4 to the default round of 3x3x3, it starts at 50% (3x3x3 scrambles are fast), goes to 100% as the 4x4x4 scrambler initializes, then jumps back to 50% when the 4x4x4 scrambles are actually being generated.

Would it be reasonable to trat the initialization as a scramble? The 4x4x4 initialization could be weighted as, say, 10 scrambles.

clementgallet commented 12 years ago

As a more general issue, when we add a second 3x3 round, the scrambling bar starts at 50% because the first 3x3 round was already generated. If we add a third one, the bar will start at 67%. Couldn't we change so that the scrambling bar starts at 0% when asking for new scrambles ?

lgarron commented 12 years ago

Wouldn't it be inefficient to regenerate all the scrambles?

Or do you mean that it should keep the old ones but start reporting the remaining scrambles?

jfly commented 12 years ago

Right now, there are two progress bars. One reflects the initialization progress of all the puzzles we care about. The other reflects the progress of all the scrambles we need. It seems very odd to me to show a progress bar for only the remaining work (it would always be at 0% !) Weighting puzzle initialization and scramble generation by how long they should take would even out the progress bar's growth, but seems really difficult with little benefit, and it just obscures things. If we're unhappy with progress bars that move at non constant velocities, then we should take the time and effort to reflect scramble generation and puzzle initialization on a per puzzle basis, rather than munging it all together. I believe this would just require changes to the ui, and no backend work.

I finally pulled in the latest changes from cubing/tnoodle. A couple comments:

cs0x7f commented 12 years ago

There're 8 types of error: * Error 1: There is not exactly one facelet of each colour * Error 2: Not all 12 edges exist exactly once * Error 3: Flip error: One edge has to be flipped * Error 4: Not all corners exist exactly once * Error 5: Twist error: One corner has to be twisted * Error 6: Parity error: Two corners or two edges have to be exchanged * Error 7: No solution exists for the given maxDepth * Error 8: Timeout, no solution within given time Will change them to java exception classes be better? (add 8 exception classes to represent each error, or any other solution?)

jfly commented 12 years ago

I'll leave it up to you.

If I were to do it, I would probably make Error 7 and Error 8 something like NoSolutionException and TimeoutException, respectively. It seems like Errors 1-6 can all be InvalidCubeException, with different error string descriptors.

cs0x7f commented 12 years ago

In my opinion, the current way(returning error string) is ok. Because:

  1. These errors should be regarded as message or solution to the given cube instead of unexpected errors.
  2. The throwing procedure might reduce the performance.
  3. Much more codes, classes which are unnecessary will be added to the project, and will cost more memory.
jfly commented 12 years ago
  1. Ok, if you feel this way, I could agree with you on ever error except for error 8. From machine to machine, I can imagine that the same starting state will result in error 8, or a different solution. It's not deterministic.
  2. Throwing will reduce the performance even when an exception is not thrown? I don't know how java exceptions are implemented, so I can't really speculate on it.
  3. Well, "unnecessary" is debatable. What sort of memory increase are we talking about?
cs0x7f commented 12 years ago

In a word, I haven't realized any advantage of using exception, especially when I try to translate it to other platforms(c, javascript, etc.). If you think it actually better, you may create a new branch.

About performance, some tests reveal that: create a normal object: 575817ns create an exception object: 9589080ns create, throw and catch an exception object: 47394475ns The fact is, the cost of creating an exception object is 20 times as normal one, and throw/catch spends much more.

jfly commented 12 years ago

The advantage of using exceptions is that your compiler will check that you've handled all the things that can go wrong when calling search(). It also lets you separate your error handling code from your everything-went-fine code. That said, I certainly don't feel strongly enough about this to make these changes and have to deal with 2 versions of each of your solvers.

Porting to other languages is a good point I had not considered.

Your stats do illustrate an important point, exceptions should not be used in the course of normal execution of code, as they are slower. However, I feel that Errors 1-6 are all indicative of bugs with whoever is calling search(), and slowing things down there is not a problem. Errors 7 and 8 only occur when things are taking a long time anyways, so I think the overhead of throwing an exception isn't a problem.

I don't need you to respond to this. Lucas, would you close this issue if we're done cleaning threephase integration?