|
Created on 2016-05-19.12:16:50 by silvan, last changed by silvan.
msg5409 (view) |
Author: silvan |
Date: 2016-05-23.11:06:25 |
|
We definitely should do so. And the best solution is probably to get rid of
Heuristic::initialize, so search time does not take into account the
initialization of any heuristics still using this method.
I merged the issue.
|
msg5408 (view) |
Author: malte |
Date: 2016-05-23.11:03:27 |
|
OK, sounds good! At some point we should clean up all the initialization vs.
construction vs. "what is search time" mess, but not in this issue. :-)
|
msg5407 (view) |
Author: silvan |
Date: 2016-05-23.10:42:36 |
|
I ran another experiment with DFP configs, just as a security check (building
the merge-and-shrink heuristic at construction time rather than at
initialization time; small polishing in DFP):
http://ai.cs.unibas.ch/_tmp_files/sieverss/issue658-v2-dfp-issue658-base-issue658-v2-compare.html
I guess the time gains (score_search_time_average) are due to what we define as
"search_time": now that the construction of merge-and-shrink does not happen
after search has started, it doesn't count towards search time anymore.
|
msg5406 (view) |
Author: malte |
Date: 2016-05-22.18:56:14 |
|
No time; please clean up and merge!
|
msg5405 (view) |
Author: silvan |
Date: 2016-05-22.18:49:22 |
|
Right. Did you plan to have a look at the second pull request for the new files,
or should I just go do the remaining clean-up and merge?
|
msg5404 (view) |
Author: malte |
Date: 2016-05-22.18:23:26 |
|
If I understand the comments on bitbucket correctly, only one of these questions
remained, which is now answered.
|
msg5403 (view) |
Author: silvan |
Date: 2016-05-22.18:02:40 |
|
I adapted the changes suggested in the code review, but I had two more questions
(see bitbucket).
|
msg5397 (view) |
Author: malte |
Date: 2016-05-21.22:29:10 |
|
Looks good to me.
|
msg5396 (view) |
Author: silvan |
Date: 2016-05-21.21:05:22 |
|
I already ran an experiment, I just didn't post it yet because I didn't look at
it in details:
http://ai.cs.unibas.ch/_tmp_files/sieverss/issue658-v1-issue658-base-issue658-v1-compare.html
Coverage looks ok, there are some memory fluctuations which may seem to be a bit
larger than usually, but maybe this is ok, given that we use new classes and
re-distributed data structure over different classes.
|
msg5395 (view) |
Author: silvan |
Date: 2016-05-21.21:03:05 |
|
Well, there can always be something that goes slightly wrong :-)
In case you would like to compare the factories against the old strategies, here
is a diff: https://bitbucket.org/SilvanS/dev-tmp/pull-requests/1/issue658/diff
|
msg5394 (view) |
Author: malte |
Date: 2016-05-21.20:52:10 |
|
No, if you think the changes are fine, I don't think it's necessary to do that.
Let's just run an experiment and then merge.
|
msg5393 (view) |
Author: silvan |
Date: 2016-05-21.20:49:56 |
|
Thanks so far.
The only way of getting a diff to the factories that I can think of is to copy
the repository, and changing the merge strategies to be the factories in that
variant. Should I do this?
|
msg5388 (view) |
Author: malte |
Date: 2016-05-21.18:21:41 |
|
I had a look at the code so far, but not at the new source files, as it's too
hard to compare the old to the new code. I'll just trust that they are all fine.
|
msg5382 (view) |
Author: silvan |
Date: 2016-05-20.11:50:59 |
|
Here is a pull request:
https://bitbucket.org/SilvanS/fd-dev/pull-requests/16/issue658
Basically, everything that was computed in the initialize-methods of merge
strategies is now computed in the factory, when compute_merge_strategy is
called. All options of merge strategies needed to be moved to the factories (and
hence, dumping the chosen options for the log file is also in the responsibility
of the factories now).
|
msg5375 (view) |
Author: malte |
Date: 2016-05-19.13:16:21 |
|
Sounds good!
|
msg5374 (view) |
Author: silvan |
Date: 2016-05-19.12:16:50 |
|
This is part of meta issue567. When I wanted to start working on issue644, I
realized that with the current way of parsing merge strategies from the command
line options, it is rather impossible to find a good way to pass a reference to
a factored transition system to a merge strategy when initializing it. Also
because in issue644, there will be more layers of classes involved in future
merge strategies, it could be beneficial to introduce a merge strategy factory
for every merge strategy that creates the resulting merge strategy object (and
all required nested classes).
|
|
Date |
User |
Action |
Args |
2016-05-23 11:06:25 | silvan | set | status: in-progress -> resolved messages:
+ msg5409 |
2016-05-23 11:03:27 | malte | set | messages:
+ msg5408 |
2016-05-23 10:42:36 | silvan | set | messages:
+ msg5407 |
2016-05-22 18:56:14 | malte | set | messages:
+ msg5406 |
2016-05-22 18:49:22 | silvan | set | messages:
+ msg5405 |
2016-05-22 18:23:26 | malte | set | messages:
+ msg5404 |
2016-05-22 18:02:40 | silvan | set | messages:
+ msg5403 |
2016-05-21 22:29:10 | malte | set | messages:
+ msg5397 |
2016-05-21 21:05:22 | silvan | set | messages:
+ msg5396 |
2016-05-21 21:03:05 | silvan | set | messages:
+ msg5395 |
2016-05-21 20:52:10 | malte | set | messages:
+ msg5394 |
2016-05-21 20:49:56 | silvan | set | messages:
+ msg5393 |
2016-05-21 18:21:41 | malte | set | messages:
+ msg5388 |
2016-05-20 11:50:59 | silvan | set | status: chatting -> in-progress messages:
+ msg5382 |
2016-05-19 13:16:21 | malte | set | messages:
+ msg5375 |
2016-05-19 12:16:50 | silvan | create | |
|