Issue906

Title Change Pattern(Collection)Information to transfer ownership rather than sharing it
Priority wish Status chatting
Superseder Nosy List florian, jendrik, malte, silvan
Assigned To silvan Keywords
Optional summary
Pull request: https://github.com/aibasel/downward/pull/9

Created on 2019-03-08.13:41:12 by silvan, last changed by florian.

Summary
Pull request: https://github.com/aibasel/downward/pull/9
Messages
msg11654 (view) Author: florian Date: 2024-07-19.13:40:54
We looked into this issue briefly and it seems the pull request is no longer correct: the only change there is removing the TaskProxy from the class, which is issue911. Silvan, are you still interested in this change? If not, I suggest we close it and the associated pull request.

If you want to continue with this, we should discuss the point mentioned by Malte in msg8656.
msg8662 (view) Author: malte Date: 2019-03-11.12:27:11
> PatternInformation may only have the pattern and no PDB. It is intended to
> compute the PDB on demand, if the user needs it.

I know; 1:1 is not quite right; it's a "1 : zero-or-1" relationship. In those
cases, the comments apply even more, though.
msg8661 (view) Author: silvan Date: 2019-03-11.09:43:41
PatternInformation may only have the pattern and no PDB. It is intended to
compute the PDB on demand, if the user needs it.

I can start working on this issue, simply ignoring the fact that we potentially
store patterns twice, since this is what we currently do already.
msg8656 (view) Author: malte Date: 2019-03-08.17:22:33
A shared pointer makes more sense when there is a dynamic web of ownership that
is orchestrated at runtime. In this case, if I understand correctly, there would
be a 1:1 correspondence between PatternInformation and PDB, and they both want
access to the pattern. In situations with such "sister classes" X and Y it is
usually desirable to either have one own the other or merge them into a single
class. In either case, we would end up with a single place in which the pattern
lives.

I cannot meaningfully comment on how the responsibilities of these classes
should be divided, if one of them could be made a more internal class, etc.,
without spending a lot more time with the code. If it is possible to proceed
with this issue without having resolved this point, I think we should do that.
We can then look into PatternInformation vs. PDB in more detail later.
msg8652 (view) Author: silvan Date: 2019-03-08.13:41:12
As discussed live yesterday and after having introduced PatternInformation next
to PatternCollectionInformation (issue892), we would like to keep patterns and
their corresponding PDBs together. This will allow making changes to a pattern
collection, such as sorting or removing elements, while keeping the list of PDBs
valid, which is also important to handle issue785 (consistent normalization of
pattern collections) easily.

The current idea is as follows: PatternCollectionInformation stores a list of
PatternInformation objects, which are a wrapper for pairs of patterns and PDBs.
(To actually have this light-weight wrapper, we should get rid of TaskProxy in
PatternInformation and let callers to "get_pdb" pass the task proxy in.) Instead
of using shared_ptr for both the pattern collection (list of patterns) and
individual PDBs insides a vector, we should consider to store the plain datatype
in the information objects. This way, they act as an interface for transferring
the patterns (and possibly PDBs and max. additive subsets) from the generator to
the user. The user would then extract the things it needs to store, thus taking
full ownership of patterns and PDBs.

This would still leave us with storing patterns twice: as the pattern of
interest (we might not have a PDB for it yet), and within the PDB, which it
needs to compute the hash function. I don't see an easy way around this except
for sharing the pattern of a PDB via shared_ptr. I don't know if that is a good
idea. What do you think?
History
Date User Action Args
2024-07-19 13:40:54floriansetmessages: + msg11654
2020-07-24 11:25:06silvansetsummary: Pull request: https://github.com/aibasel/downward/pull/9
2019-03-18 09:21:24floriansetnosy: + florian
2019-03-11 12:27:11maltesetmessages: + msg8662
2019-03-11 09:43:41silvansetmessages: + msg8661
2019-03-08 17:22:33maltesetmessages: + msg8656
2019-03-08 13:41:12silvancreate