|
Created on 2015-04-28.10:51:53 by pvonreth, last changed by jendrik.
msg7610 (view) |
Author: jendrik |
Date: 2018-09-19.20:23:43 |
|
I created issue834 for the tasks in msg7602.
|
msg7602 (view) |
Author: malte |
Date: 2018-09-19.19:20:46 |
|
Great! :-) In a separate issue (which someone needs to create), we can now
remove the traces of the old class Plugin from the code. I suggest doing this in
the following steps, which in my opinion can be done in the same issue, but I
would do them in separate commits:
1. Remove code related to the class Plugin.
I think all this code should be in the option parser subdirectory.
It is not enough to search for "Plugin". Also look for overloads of functions
or templates that have a version for shared pointers and another one for
raw pointers. Searching for the word "shared" in the option parser should
probably find most or all relevant places: the code for raw pointers should
be right next to the code for shared pointers.
2. Optionally, clean up the method names etc. in the option parser that
previously included the word "shared" to distinguish them from the
raw pointer versions.
3. Rename the class PluginShared to Plugin. (This will touch lots of source
files, so should be a commit of its own.)
There is alread a task on the taskboard for this step, but I think there isn't
an issue yet.
|
msg7594 (view) |
Author: manuel |
Date: 2018-09-19.17:32:06 |
|
Merged and pushed.
|
msg7582 (view) |
Author: malte |
Date: 2018-09-19.16:14:24 |
|
The behaviour on nomystery-sat11, instance p06, looks odd for lama-lazy. But I
think this is not related to this issue, and I'm happy to see this merged.
|
msg7578 (view) |
Author: manuel |
Date: 2018-09-19.15:54:04 |
|
I run experiments on configurations
- ehc with ff and ff's preferred operators,
- lama-first, and
- lama-first with eager greedy instead of lazy greedy.
You find the result here:
https://ai.dmi.unibas.ch/_tmp_files/heusner/issue526-v1-sat-issue526-base-issue526-v1-compare.html
The results show no unexpected artifacts.
|
msg7559 (view) |
Author: jendrik |
Date: 2018-09-19.11:17:10 |
|
I had a look and left two minor comments.
|
msg7540 (view) |
Author: malte |
Date: 2018-09-18.19:29:28 |
|
Done reviewing, looks good! As commented on bitbucket, I think I found a subtle
bug where preferred operators in EHC are ignored.
These sorts of things creep in easily, so I think it would be better if we ran
some experiments to make sure we didn't overlook anything.
5-minute experiments should be enough, but they should exercise all three search
algorithms (lazy, greedy, ehc), and they should include cases with preferred
operators from more than one heuristic because these are the ones most affected
by the change.
I think it's enough to do satisficing experiments. lama-first is a good test for
lazy; one of the non-lazy configs of lama is a good test for greedy. For ehc
just use one configuration with ehc, for example using the FF heuristic both as
a heuristic and for preferred operators.
Once this is merged, I'd be interested in seeing how many parts of the planner
still use "new" and/or "delete" explicitly. We should be well on our way towards
our goal of having well-defined lifetimes for everything.
|
msg7531 (view) |
Author: jendrik |
Date: 2018-09-18.18:17:55 |
|
Pull request: https://bitbucket.org/gutoblaas/downward-issue526/pull-requests/1
|
msg7505 (view) |
Author: guillem |
Date: 2018-09-18.10:55:43 |
|
I had a look at the PR; there are a few calls to
EvaluatorContext::get_evaluator_value(evaluator.get()). Once we start moving to smart
pointers, I'd personally try to avoid moving the same pointer around both as a "smart" and
"non-smart" version *at the same time*, as it confuses pointer ownership and can lead to
obscure bugs (everything is fine with this code, just thinking ahead). In this case, we
could perhaps refactor the "get_evaluator_value" to accept a Evaluator&, and only in the
EvaluatorCache convert that into a pointer? (one could also use a map of
std::reference_wrapper, but we probably don't want to pay the overhead in this particular
place of the code).
Just a suggestion to think about!
|
msg7481 (view) |
Author: manuel |
Date: 2018-09-17.14:12:56 |
|
We will use shared_ptr for all plugins.
|
msg6929 (view) |
Author: jendrik |
Date: 2018-03-16.19:25:26 |
|
It's probably best to wait with moving forward here until issue766 and issue718
are merged, since they touch the same portions of code.
|
msg6800 (view) |
Author: malte |
Date: 2018-02-09.16:25:38 |
|
Is there still a difference between Heuristic and Evaluator? If yes, then the
issue for unifying them is still open, and working on that issue at the same
time as this one will likely lead to substantial merge conflicts.
I would recommend having a brief look at the status of the Heuristic/Evaluator
unification issue before proceeding here.
|
msg6799 (view) |
Author: guillem |
Date: 2018-02-09.16:23:02 |
|
I'd like to push this one forward. As per the FD meeting yesterday, there should
be no big obstacle, so I'll probably have a go at this in the next days. Although
not mentioned in the title, I assume I could tackle using shared_ptr's for
Evaluators as well?
|
|
Date |
User |
Action |
Args |
2018-09-19 20:23:43 | jendrik | set | messages:
+ msg7610 summary: Once this issue is done, rename PluginShared to Plugin. -> |
2018-09-19 19:20:46 | malte | set | messages:
+ msg7602 |
2018-09-19 17:32:06 | manuel | set | status: in-progress -> resolved nosy:
+ augusto messages:
+ msg7594 |
2018-09-19 16:14:24 | malte | set | messages:
+ msg7582 |
2018-09-19 15:54:04 | manuel | set | messages:
+ msg7578 |
2018-09-19 11:17:10 | jendrik | set | messages:
+ msg7559 |
2018-09-18 19:29:28 | malte | set | messages:
+ msg7540 |
2018-09-18 18:17:55 | jendrik | set | messages:
+ msg7531 summary: Blocked by issue766 and issue718.
Once this issue is done, rename PluginShared to Plugin. -> Once this issue is done, rename PluginShared to Plugin. |
2018-09-18 10:55:43 | guillem | set | messages:
+ msg7505 |
2018-09-17 14:12:56 | manuel | set | nosy:
+ manuel messages:
+ msg7481 |
2018-04-04 10:58:15 | jendrik | set | summary: Blocked by issue766 and issue718. -> Blocked by issue766 and issue718.
Once this issue is done, rename PluginShared to Plugin. |
2018-03-16 19:25:26 | jendrik | set | messages:
+ msg6929 summary: Blocked by issue766 and issue718. |
2018-02-09 16:25:38 | malte | set | nosy:
- pvonreth messages:
+ msg6800 |
2018-02-09 16:23:02 | guillem | set | assignedto: pvonreth -> guillem messages:
+ msg6799 nosy:
+ guillem |
2015-04-28 10:51:53 | pvonreth | create | |
|