Issue524

Title use shared_ptr for landmark graphs
Priority feature Status resolved
Superseder Nosy List cedric, florian, jendrik, malte, silvan
Assigned To cedric Keywords
Optional summary

Created on 2015-04-21.10:19:41 by pvonreth, last changed by cedric.

Messages
msg7000 (view) Author: cedric Date: 2018-04-04.10:50:10
Code is merged.
msg6965 (view) Author: jendrik Date: 2018-03-31.19:43:54
The code looks good to me.
msg6964 (view) Author: malte Date: 2018-03-31.17:44:27
Experiments look good to me. If the others are happy with the code, this is
ready to be merged.
msg6947 (view) Author: cedric Date: 2018-03-19.15:23:11
Here is the new comparison report:
http://ai.cs.unibas.ch/_tmp_files/geissmann/issue524-v2-issue524-base-v2-issue524-v2-compare.html

The absolute report is updated.
msg6902 (view) Author: silvan Date: 2018-03-16.15:19:44
Add a tag issue524-base-v2 to the default changeset which was last merged into
the issue branch.
msg6901 (view) Author: malte Date: 2018-03-16.15:15:34
The effect of this on performance are much larger than they should be. They
should be essentially imperceptible. I think the reason is that the branch
merged from default, but the experiment compares to an older base version from
default. Therefore, issue524-base vs. issue524-v2 includes several changes from
other issues in addition to the change from this issue.

People on the nosy list, what is our best practice for this? Update the
issue524-base tag, or introduce a new tag for the correct reference version?
msg6897 (view) Author: cedric Date: 2018-03-16.14:56:23
Here are the results as a comparison report:
http://ai.cs.unibas.ch/_tmp_files/geissmann/issue524-v2-issue524-base-issue524-v2-compare.html

And the absolute report can be found here:
http://ai.cs.unibas.ch/_tmp_files/geissmann/issue524-v2.html
msg6872 (view) Author: malte Date: 2018-03-15.19:35:21
Yes, I would like to add lama-first so that we also exercise the code path that
uses this from the (new) synergy heuristics.
msg6869 (view) Author: jendrik Date: 2018-03-15.17:25:03
The code looks good to me. I think you could add lama-first to the existing 
experiment before running it.
msg6867 (view) Author: malte Date: 2018-03-15.16:46:30
Yes, please!
msg6866 (view) Author: cedric Date: 2018-03-15.16:36:57
I merged the current default into this branch to be up-to-date. Maybe some can
have a look at the pull request.

Should I run some experiments for this issue?
msg6650 (view) Author: malte Date: 2017-11-30.12:16:05
Let me know if/when I should review the code. But if Jendrik has already
commented, it's probably a good idea to deal with his comments first. I hope
mine won't be contradictory. :-)
msg6648 (view) Author: jendrik Date: 2017-11-30.12:05:31
I left some comments on bitbucket.
msg6647 (view) Author: cedric Date: 2017-11-30.11:52:10
Changed to shared_ptr for landmark graphs.

Here is the pull request:
https://bitbucket.org/cgeissmann/downward/pull-requests/5/issue524/diff
History
Date User Action Args
2018-04-04 10:50:11cedricsetstatus: chatting -> resolved
messages: + msg7000
2018-03-31 19:43:54jendriksetmessages: + msg6965
2018-03-31 17:44:27maltesetmessages: + msg6964
2018-03-19 15:23:11cedricsetmessages: + msg6947
2018-03-16 15:19:44silvansetmessages: + msg6902
2018-03-16 15:15:34maltesetmessages: + msg6901
2018-03-16 14:56:23cedricsetmessages: + msg6897
2018-03-15 19:35:21maltesetmessages: + msg6872
2018-03-15 17:25:03jendriksetmessages: + msg6869
2018-03-15 16:46:30maltesetmessages: + msg6867
2018-03-15 16:36:57cedricsetmessages: + msg6866
2017-11-30 12:16:05maltesetmessages: + msg6650
2017-11-30 12:05:31jendriksetmessages: + msg6648
2017-11-30 11:52:10cedricsetassignedto: cedric
messages: + msg6647
nosy: + cedric
2017-04-27 14:57:45jendriksetnosy: - pvonreth
2017-04-27 14:55:38jendriksetassignedto: pvonreth -> (no value)
2015-04-21 10:19:41pvonrethcreate