Ticket #220 (closed defect: fixed)
start_sgftrace changes OWL result
| Reported by: | martin | Owned by: | gnugo |
|---|---|---|---|
| Priority: | normal | Milestone: | 3.9.1 |
| Component: | source | Version: | |
| Severity: | normal | Keywords: | |
| Cc: | patch: | yes |
Description
Invoking start_sgftrace may alter the result of OWL reading, as exemplified by the attached files. Both test1.tst and test2.tst load the same test.sgf and perform OWL attack reading on Q5. In test2.tst, start_sgftrace is called beforehand and here, gnugo succeeds in finding the attacking move at Q6, while in test1.tst, it fails.
Interestingly,
gnugo -l test.sgf --decide-dragon Q5
also gives the correct result Q6, independently of any tracing.
Attachments
Regression Results
| Attachment | Rev. | PASS | FAIL | Nodes | Status | |
| martin_9_1.5.diff | never tested |
Change History
comment:1 follow-up: ↓ 2 Changed 22 months ago by martin
As the correct move is found by tactical reading, I have also tried replacing owl_attack with attack; in that case Q6 is found with or without start_sgftrace. So the problem seems to be somewhere in the OWL code where the tactical reading is invoked or where its move is tried.
comment:2 in reply to: ↑ 1 Changed 22 months ago by martin
Replying to martin:
As the correct move is found by tactical reading, I have also tried replacing owl_attack with attack; in that case Q6 is found with or without start_sgftrace. So the problem seems to be somewhere in the OWL code where the tactical reading is invoked or where its move is tried.
Correction: The situation where the correct move is found by tactical reading is two moves into the OWL reading, so doing attack on the original position does not provide any useful information.
comment:3 Changed 22 months ago by martin
By comparing the trace of start_sgftrace and the output with -t but without start_sgftrace, the difference seems to start in the first variation after w S7 b Q6. With start_sgftrace, a save lunch move is generated at S8; without start_sgftrace, instead pattern A811 is matched, indicating that the R7 string is considered dead.
comment:4 follow-up: ↓ 5 Changed 22 months ago by gunnar
This seems odd. Have you tried running it under valgrind?
comment:5 in reply to: ↑ 4 Changed 22 months ago by martin
Replying to gunnar:
This seems odd. Have you tried running it under valgrind?
I have, but everything looked good.
I have also tried running a few of the regression testsuites with an added start_sgftrace at the top. Those which do not involve OWL reading (reading, optics, connection, unconditional) did not change at all, while some of the others showed an increase in node counts (e.g. owl: +2.2% +0.6% +0.04%). None of the ten or so testsuites I ran had any different results, though.
comment:6 Changed 21 months ago by martin
Comparing execution with and without start_sgftrace, the point where the reading changes in terms of the reading node counter is in do_owl_attack -> do_owl_defend -> owl_update_goal -> find_superstring -> do_find_superstring -> attack -> do_attack.
If called without start_sgftrace, do_attack finds a result in the cache, while with start_sgftrace, it does not, invoking attack3. The result returned by attack3 is the same as the cached one, but the fact that the content of the cache depends on whether tracing is enabled does seem a bit fishy.
comment:7 follow-up: ↓ 9 Changed 21 months ago by martin
- patch set
The reason for the different results is a reading_cache_clear() between silent_examine_position() and owl_attack(). In gtp_owl_attack(), it is conditioned on sgf_dumptree, in decide_owl() it is unconditional. In gtp_owl_attack(), the comment before reading_cache_clear() says "to get the variations into the sgf file, clear the reading cache". However, whether the is cleared or nor, only the OWL reading, not the tactical reading is traced into the sgf file, so clearing the cache does not seem to do anything helpful. The same is probably true for decide_owl().
The appended patch removes all those calls to reading_cache_clear() between silent_examine_position() and the OWL functions. If anyone knows a good reason for clearing the cache at these points, we will have to find some solution which at least does not change the behavior depending on whether tracing is on and should make --decide-dragon and owl_attack/owl_defend give the same result.
comment:9 in reply to: ↑ 7 Changed 18 months ago by gunnar
Replying to martin:
The reason for the different results is a reading_cache_clear() between silent_examine_position() and owl_attack(). In gtp_owl_attack(), it is conditioned on sgf_dumptree, in decide_owl() it is unconditional.
That's probably historical. When you call decide_owl() that's the only thing you ever do so then it makes sense to prioritize having the same results with and without sgf tracing. The newer gtp functions can be mixed up with lot of other things and you don't want to lose cache unnecessarily.
In gtp_owl_attack(), the comment before reading_cache_clear() says "to get the variations into the sgf file, clear the reading cache". However, whether the is cleared or nor, only the OWL reading, not the tactical reading is traced into the sgf file, so clearing the cache does not seem to do anything helpful.
On the contrary, the reading cache contains not only tactical reading results but also connection reading, owl reading, and semeai reading results. Since you don't know what has preceded the gtp_owl_attack() call the cache can contain anything, in particular if a reg_genmove was done before the owl reading.
The same is probably true for decide_owl().
This is different. Here nothing else can happen before (only called when running --decide-owl) so probably the first call is ineffective with respect to owl reading. The second call on the other hand may make a difference to avoid that the defense reading picks up cached results from the attack reading. (Normally they are out of phase with respect to color to move but that may change after a pass.)
The appended patch removes all those calls to reading_cache_clear() between silent_examine_position() and the OWL functions. If anyone knows a good reason for clearing the cache at these points, we will have to find some solution which at least does not change the behavior depending on whether tracing is on and should make --decide-dragon and owl_attack/owl_defend give the same result.
I distinctly remember having ended up with pointless sgf files only telling a result from the cache, so it may be that I added the cache clearing code in the first place. This is all way back in the mists of time, however, so maybe we can try to do it differently now. In particular there is now a gtp command to clear the cache explicitly, which should be more flexible.
comment:10 Changed 14 months ago by gunnar
Apparently view.pike is already issuing clear_cache commands, so there it shouldn't be a problem. Let's try and see whether any problems turn up.
comment:11 Changed 14 months ago by gunnar
- Status changed from new to closed
- Resolution set to fixed

