Created on 2015-10-28.15:33:41 by malte, last changed by malte.
1. Get rid of the need to define TokenParser::parse in option_parser.h.
2. Get rid of manual calls to get_help_templ() in two places in
option_parser.cc. => deferred; see issue588.
3. Get rid of need to define a TypeNamer for the new type in
option_parser_util.h.
4. Get rid of the TypeDocumenter specializations in option_parser_util.h.
Before merging, grep the code for "issue586" to make sure all TODOs marked in
the code are dealt with. => some TODOs remain that are now marked as
"post-issue586". These will be addressed in issue588.
|
msg4756 (view) |
Author: malte |
Date: 2015-11-10.12:57:13 |
|
Yes, I will close issue286 and also issue266 referenced from there. Thanks for
the pointer!
|
msg4754 (view) |
Author: florian |
Date: 2015-11-10.11:49:29 |
|
Does this also resolve issue286?
|
msg4728 (view) |
Author: malte |
Date: 2015-11-01.12:01:06 |
|
> If you want to see the the wiki-formatted text, you could import and call
> build_planner() and get_pages_from_planner() from autodoc.py.
That's effectively what I did.
|
msg4727 (view) |
Author: florian |
Date: 2015-11-01.00:02:50 |
|
For the record: you can locally test the wiki output with the options "--help
--txt2tags" but this is unformatted txt2tags code. If you want to see the the
wiki-formatted text, you could import and call build_planner() and
get_pages_from_planner() from autodoc.py. Alternatively, we could add a dry-run
option to autodoc, that stops the scripts before uploading.
|
msg4725 (view) |
Author: malte |
Date: 2015-10-31.21:04:03 |
|
OK, it was indeed easy to fix. Marking as resolved.
|
msg4724 (view) |
Author: malte |
Date: 2015-10-31.18:47:50 |
|
Merged. The followup issue is issue588.
However, the buildbot does complain. At first glance, it looks like this is
because we now verify that we don't insert duplicate keys into plugin
registries, and apparently the open list registration code does introduce
duplicate entries. If this is as easy to fix as I hope, I'll do it directly in
the default branch.
|
msg4722 (view) |
Author: malte |
Date: 2015-10-31.18:30:52 |
|
Done, but the real test will be what happens to the auto-docs once this is
merged. The further changes after Florian's review were pretty much
straight-forward, so I hope it's OK if I merge this without further review
(assuming that Florian is currently travelling).
Working with the option parser code reminded me of some open issues there, and
it's perhaps a good idea to open a meta issue for that.
|
msg4721 (view) |
Author: malte |
Date: 2015-10-31.14:00:57 |
|
Thanks for the feedback, Florian!
The basic code for 4. is now also done, but it's a bit hard to test this because
the TypeDocumenter doesn't affect the regular --help output, only what is
printed in "wiki mode", and I have yet to find out how to test the wiki mode
output offline.
Still, this is mostly done. The main work that still remains to be done is
actually using the new plugin type plugins.
|
msg4716 (view) |
Author: malte |
Date: 2015-10-30.22:33:34 |
|
Here is a link to the pull request:
https://bitbucket.org/malte/downward/pull-requests/2/issue586/diff
|
msg4715 (view) |
Author: malte |
Date: 2015-10-30.22:30:56 |
|
I've pushed some changes that address 3 and made a pull request because now
might be a good opportunity for feedback. There are many TODOs in the code, but
all the main pieces for 1. and 3. are in place, and 4. should be mechanical given 3.
The main work that isn't done yet is actually introducing most of the
plugin-type plugins -- I wanted to wait with this until we're more certain of
the design.
|
msg4713 (view) |
Author: florian |
Date: 2015-10-30.18:54:40 |
|
Sounds good to me. I prefer having multiple small issues to one large one.
|
msg4710 (view) |
Author: malte |
Date: 2015-10-30.18:37:14 |
|
Having worked on this some more, 3. and 4. are reasonably easy, but 2. is
tricky. There is a lot of code triggered on that code path which currently
depends on compile-time knowledge of the types in question and would need to be
converted to something that only knows the types at runtime. This would require
more major restructuring of the option parser code than I expected.
Perhaps it would be better to first merge only the changes for 1., 3. and 4 and
leave 2. to a separate issue so that this one can be merged more quickly and is
more reviewable.
|
msg4707 (view) |
Author: malte |
Date: 2015-10-30.18:21:21 |
|
Looks like I accidentally classified this as a bug. Fixed.
|
msg4688 (view) |
Author: malte |
Date: 2015-10-28.18:50:24 |
|
Ah, there's also optional documentation for the plug-in type that we don't
always use but should definitely support (TypeDocumenter). I've added a TODO
list to the summary.
|
msg4687 (view) |
Author: malte |
Date: 2015-10-28.18:46:51 |
|
I've started working on the code for the non-predefinable case. There are
currently three things that need to be done when adding a new type to the option
parser.
1. Define TokenParser::parse for the new type in option_parser.h.
2. Call get_help_templ() for the new type in two places in option_parser.cc.
3. Define a TypeNamer for the new type in option_parser_util.h.
Step 1. is done in the issue branch
(https://bitbucket.org/malte/downward/branch/issue586). The other two steps are
only needed for the help output, but since this generates our wiki
documentation, this is of course important, too. For this we need a new kind of
plug-in. I think it won't be very complicated, but it may take me a while to
find time for it.
|
msg4686 (view) |
Author: malte |
Date: 2015-10-28.17:48:44 |
|
I looked into this a bit. Some things look easy, some not so easy. I'll try to
describe some of the issues involved.
Firstly, we have a distinction between "predefinable" plug-in types and ones
that are not predefinable.
Predefinable types can be "assigned to a variable" on the command line, as in
"--heuristic h=ff()". These can then be used in multiple places. For example, we
can use "h" both in a list of heuristics to evaluate and in a list of heuristics
to use for preferred operators. The only plug-in types that are currently
predefinable are heuristics (plus scalar evaluators, see below) and landmark
factories.
It is not complicated to make additional plug-in types predefinable, but our
code is not prepared for it because the internal state of the objects assumes
that it is used in only one context. For example, using the same merge strategy
object within two different merge-and-shrink heuristic objects is not going to
work well because it is likely to have internal state that is related to its
unique user. Given this, I'll start by only making the non-predefinable plugin
types fully pluggable. This is the simpler and more common case. We can later
make predefinable plugin types pluggable.
Secondly, inheritance complicates the plug-in code. In places where we currently
accept scalar evaluators, we must make sure to support both "ScalarEvaluator"
plugins and "Heuristic" plug-ins. We have no general mechanism for this: the
option parser code directly knows the necessary information about the
inheritance hierarchy and contains code to handle it correctly.
I think the cost of this feature is higher than its utility. Actually, I think
the utility is negative because exposing this inheritance relationship to the
user complicates the documentation etc. So my suggestion is to get rid of
inherited plugin types in the medium term. The alternative would be to
strengthen the support for inheritance between plugin types (which would
complicate this issue, but so be it).
|
msg4685 (view) |
Author: malte |
Date: 2015-10-28.15:33:41 |
|
"Plug-ins" are currently fully pluggable: for example, we can add a new
heuristic, shrink strategy or landmark generation method without touching any
code, only adding new source files.
"Plug-in types" are currently not pluggable: if we want to add a new *kind* of
plug-in, we need to touch various places in the option parser code. For example,
we had to do this when we added plug-ins for shrink strategies, merge
strategies, labels and (in issue527, not merged yet) constraint generators.
The suggestion is to change the option parser code so that plug-in types can be
added in a decentralized fashion.
|
|
Date |
User |
Action |
Args |
2015-11-10 12:57:13 | malte | set | messages:
+ msg4756 |
2015-11-10 11:49:29 | florian | set | messages:
+ msg4754 |
2015-11-01 12:01:06 | malte | set | messages:
+ msg4728 |
2015-11-01 00:02:50 | florian | set | messages:
+ msg4727 |
2015-10-31 21:04:28 | malte | set | status: chatting -> resolved |
2015-10-31 21:04:03 | malte | set | messages:
+ msg4725 |
2015-10-31 18:47:50 | malte | set | messages:
+ msg4724 summary: 1. ~~Get rid of the need to define TokenParser::parse in option_parser.h.~~
2. Get rid of manual calls to get_help_templ() in two places in
option_parser.cc. => deferred to a later issue
3. ~~Get rid of need to define a TypeNamer for the new type in
option_parser_util.h.~~
4. ~~Get rid of the TypeDocumenter specializations in option_parser_util.h.~~
~~Before merging, grep the code for "issue586" to make sure all TODOs marked in
the code are dealt with.~~ => some TODOs remain that are now marked as
"post-issue586" -> 1. ~~Get rid of the need to define TokenParser::parse in option_parser.h.~~
2. Get rid of manual calls to get_help_templ() in two places in
option_parser.cc. => deferred; see issue588.
3. ~~Get rid of need to define a TypeNamer for the new type in
option_parser_util.h.~~
4. ~~Get rid of the TypeDocumenter specializations in option_parser_util.h.~~
~~Before merging, grep the code for "issue586" to make sure all TODOs marked in
the code are dealt with.~~ => some TODOs remain that are now marked as
"post-issue586". These will be addressed in issue588. |
2015-10-31 18:30:52 | malte | set | messages:
+ msg4722 summary: 1. ~~Get rid of the need to define TokenParser::parse in option_parser.h.~~
2. Get rid of manual calls to get_help_templ() in two places in
option_parser.cc.
3. Get rid of need to define a TypeNamer for the new type in
option_parser_util.h.
4. Get rid of the TypeDocumenter specializations in option_parser_util.h.
Before merging, grep the code for "issue586" to make sure all TODOs marked in
the code are dealt with.
For now, we only cover plugin types that don't support predefinitions.
Predefinitions will be handled later. -> 1. ~~Get rid of the need to define TokenParser::parse in option_parser.h.~~
2. Get rid of manual calls to get_help_templ() in two places in
option_parser.cc. => deferred to a later issue
3. ~~Get rid of need to define a TypeNamer for the new type in
option_parser_util.h.~~
4. ~~Get rid of the TypeDocumenter specializations in option_parser_util.h.~~
~~Before merging, grep the code for "issue586" to make sure all TODOs marked in
the code are dealt with.~~ => some TODOs remain that are now marked as
"post-issue586" |
2015-10-31 14:00:57 | malte | set | messages:
+ msg4721 |
2015-10-30 22:33:34 | malte | set | messages:
+ msg4716 |
2015-10-30 22:30:56 | malte | set | messages:
+ msg4715 summary: 1. ~~Get rid of the need to define TokenParser::parse in option_parser.h.~~
2. Get rid of manual calls to get_help_templ() in two places in
option_parser.cc.
3. Get rid of need to define a TypeNamer for the new type in
option_parser_util.h.
4. Get rid of the TypeDocumenter specializations in option_parser_util.h. -> 1. ~~Get rid of the need to define TokenParser::parse in option_parser.h.~~
2. Get rid of manual calls to get_help_templ() in two places in
option_parser.cc.
3. Get rid of need to define a TypeNamer for the new type in
option_parser_util.h.
4. Get rid of the TypeDocumenter specializations in option_parser_util.h.
Before merging, grep the code for "issue586" to make sure all TODOs marked in
the code are dealt with.
For now, we only cover plugin types that don't support predefinitions.
Predefinitions will be handled later. |
2015-10-30 18:54:40 | florian | set | messages:
+ msg4713 |
2015-10-30 18:37:14 | malte | set | messages:
+ msg4710 |
2015-10-30 18:21:21 | malte | set | priority: bug -> wish messages:
+ msg4707 |
2015-10-28 18:50:42 | malte | set | summary: 1. ~~Get rid of the need to define TokenParser::parse in option_parser.h.~~
2. ~~Get rid of manual calls to get_help_templ() in two places in
option_parser.cc.~~
3. ~~Get rid of need to define a TypeNamer for the new type in
option_parser_util.h.~~
4. ~~Get rid of the TypeDocumenter specializations in option_parser_util.h.~~ -> 1. ~~Get rid of the need to define TokenParser::parse in option_parser.h.~~
2. Get rid of manual calls to get_help_templ() in two places in
option_parser.cc.
3. Get rid of need to define a TypeNamer for the new type in
option_parser_util.h.
4. Get rid of the TypeDocumenter specializations in option_parser_util.h. |
2015-10-28 18:50:24 | malte | set | messages:
+ msg4688 summary: 1. ~~Get rid of the need to define TokenParser::parse in option_parser.h.~~
2. ~~Get rid of manual calls to get_help_templ() in two places in
option_parser.cc.~~
3. ~~Get rid of need to define a TypeNamer for the new type in
option_parser_util.h.~~
4. ~~Get rid of the TypeDocumenter specializations in option_parser_util.h.~~ |
2015-10-28 18:46:51 | malte | set | messages:
+ msg4687 |
2015-10-28 17:48:44 | malte | set | messages:
+ msg4686 |
2015-10-28 15:40:41 | jendrik | set | nosy:
+ jendrik |
2015-10-28 15:34:46 | malte | set | nosy:
+ florian |
2015-10-28 15:33:41 | malte | create | |
|