Munging object name on Presentation

[quffaro]

I am drafting this PR to resolve a two-ish year old Catlab issue 828, which desires unique pairs of homs within a schema. This feature would be useful especially in database applications, where any two tables (Obs) may have columns (homs/attrs) with the same name.

However this introduces some downstream implementation issues. For example, suppose someone wanted to define the following schema:

@present SchExample(FreeSchema) begin
    Name::AttrType
    (X, Y)::Ob
    name::Attr(X, Name)
    name::Attr(Y, Name)
end

The changes introduced by the first commit on this PR implies that since name::Attr(X, Name) is defined, rather than throwing an error it would munge the object associated to its domain. In our example, the second name becomes Y_name. However this unhappily changes data the user provides.

A slightly deeper change would be to reorganize (or add an additional field) to the internals of Presentation to group names by type. For instance (pseudocode),

# ... dictionary entries
Attr(X, Name) => [:name]
Attr(Y, Name) => [:name]

Therefore collision-checking would first check if the arguments of the expr collide, and then whether the name does. Since add_part! mutates ACSets one part at a time, there should be no issues with keyword collision, as in the case here:

add_part!(acset, :X, name=:first)
add_part!(acset, :Y, name=:second)

Are there any hazards I'm not considering?

[kris-brown]

If I understand correctly, if there's a name collision, only the second name gets renamed?

Regardless, I'm worried that the complexity / possibility for confusion by a silent name swap might outweigh the value added by doing the munging because all of your downstream code has to refer to the munged names (so, the fact that it's automatic to perform the swap doesn't seem too helpful?).

What about this? If you write a @present that has a name collision, you emit in the error message:

Hi, consider replacing your @present with

@present SchExample(FreeSchema) begin
    Name::AttrType
    (X, Y)::Ob
    X_name::Attr(X, Name)
    Y_name::Attr(Y, Name)
end

Which the user could just copy-paste in. It would require similar logic to implement.

[kris-brown]

I agree that this is an important problem and the real solution is the deeper change to support name collisions when the types are different! I'd be happy to help fix downstream catlab code that breaks due to implementing that!

[quffaro]

Thanks for reviewing @kris-brown. I agree that doing silent munging is unpleasant and too complicated/risky for the value.

I think the error message idea you proposed is the simplest way to avoid the problem. But if you don't see anything wrong at first glance with the second proposal--to change anti-collision to instead check some dictionary Dict{Attr, Vector{Symbol}), where Attr is the type signature of Attr --then I can try to prove the concept. It seems to knock wood a small lift.

[quffaro]

@kris-brown I managed to implemented collision avoidance for synonymous but type-differing generators. Broadly speaking, I moved the generator_name_index field into its own struct Generators which also has a registry field, which groups generators by type arguments. I added methods for getting/setting on this struct.

In effect, this PR lets this @present macro succeed, and I can query all reputation generators by pres[:reputation] or specifically pres[:reputation, [:Employee, :Str]] or pres[:reputation, [:Department, :Str]]

There's a few things I'd like to call attention to which prevents me from considering this PR done:

1. For the time being, Obs are represented in the registry as argument-less, but are there other 0-ary generator types? If so, I'll need to amend this to like (type, args...).

2. I'd like to get rid of the try-catch pattern I added. Currently, representing the type signature as (:Hom, :Arg1, :Arg2) is challenged for the case when Arg1 is, say, mcompose(A,B) as it is in the FreeMC test in test/models/Presentations.jl:114.
   

Background: Because we distinguish generators by argument, the generator_name_index.index field now is now keyed by a tuple of generator name and its arguments. Therefore to access this index we pass a key which is also a tuple.

Problem: The index_generators function returns a subdictionary of all matches. This changes the behavior of accessing the index, namely the generator function, which now returns all matches. Therefore getting just one result either means that (1) there is only one instance of that name in the index, or (2) the arg types are also provided, so to make the filtering specific enough.

Question: What should generators do? Return as many matches, the first match, or if there are multiple matches, throw an error?

A similar problem is noted on line 229.

Other things:

• get_arg_names is unused