Option to deterministically generate unique ids instead of randomized animal names

[philtomson]

There are a couple of problems with the current animal name scheme used in alias_gensyms:

1. There are only about 220 animal names in animals.txt. If you have more than 220 gensyms the following line in the current alias_gensyms function is going to error out:

Base.@get!(syms, x, pop!(left))

(left is a copy of the animals symbol list)

2. Currently, the animal names list is randomized when the package is initialized. While working on ONNX.jl we were finding that given the same inputs we would get different code generated due to this randomization. This makes it very difficult for us to debug as we made changes to ONNX.jl and wanted to compare code generated from a prior version on the same inputs.

This patch generates a simple unique id in the form of "sym_id_$counter" where the counter value is incremented on each gensym encountered.

I would have preferred replacing all of the "#" characters in the gensym with blanks, however that did not work for me even though I tried the same scheme in the REPL on a gensym string and it did work (see the comment I added in alias_gensyms - maybe someone could get that working?)

[ayush1999]

Changes look great to me. Moreover, they seem to have solved the issue of repeating names in ONNX. 👍 . @MikeInnes Can you merge these changes so that we can continue its usage with ONNX?

[MikeInnes]

I would prefer this was just a new function (maybe parseable_gensyms?)

I'd also like to see a test case for the repeated names if you have one; I haven't seen that happening before. I do think it makes sense to make the names a bit more stable.

[philtomson]

Sure, a completely new function would be fine, maybe call it gensyms_to_ids? I wonder, though, if it might be best just to eliminate the animal names entirely and use the generated identifiers - the animal names approach is going to cause problems when there is a large number of gensyms (greater than the number of animal names on animals.txt) and it's easy enough just to generate new unique identifiers via a counter or just eliminate the '#''s from the gensym symbol as you convert to string, though, I suppose the advantage to generating a unique id with a counter is that you'd be able to easily see the progression of identifier names and usage (the numbers on the current gensyms seem to skip around).

[philtomson]

Updated pull request; added gensyms_to_ids function which replaces gensyms with deterministically generated identifiers.

Also: checking with Ayush about the repeated names problem; I wasn't seeing it in ONNX.jl just now prior to replacing the call to alias_gensyms with gensyms_to_ids. However I do remember seeing generated code with a duplicated animal name at some point.

[ayush1999]

@MikeInnes A useful test case can be the results while generating the ONNX model file. A sample generated file is:

begin 
    sealion = Chain(Dropout(0.5f0), Conv(relu, weights["conv10_w_0"], weights["conv10_b_0"], (1, 1), (0, 0)), ((hornet,)->mean(hornet, (1, 2))), softmax)
    vicuña = Conv(relu, weights["fire9/expand1x1_w_0"], weights["fire9/expand1x1_b_0"], (1, 1), (0, 0))
    kangaroo = Conv(relu, weights["fire9/squeeze1x1_w_0"], weights["fire9/squeeze1x1_b_0"], (1, 1), (0, 0))
    narwhal = Conv(relu, weights["fire8/expand1x1_w_0"], weights["fire8/expand1x1_b_0"], (1, 1), (0, 0))
    parrot = Conv(relu, weights["fire8/squeeze1x1_w_0"], weights["fire8/squeeze1x1_b_0"], (1, 1), (0, 0))
    bear = Conv(relu, weights["fire7/expand1x1_w_0"], weights["fire7/expand1x1_b_0"], (1, 1), (0, 0))
    butterfly = Conv(relu, weights["fire7/squeeze1x1_w_0"], weights["fire7/squeeze1x1_b_0"], (1, 1), (0, 0))
    eagle = Conv(relu, weights["fire6/expand1x1_w_0"], weights["fire6/expand1x1_b_0"], (1, 1), (0, 0))
    rat = Conv(relu, weights["fire6/squeeze1x1_w_0"], weights["fire6/squeeze1x1_b_0"], (1, 1), (0, 0))
    pheasant = Conv(relu, weights["fire5/expand1x1_w_0"], weights["fire5/expand1x1_b_0"], (1, 1), (0, 0))
    koala = Conv(relu, weights["fire5/squeeze1x1_w_0"], weights["fire5/squeeze1x1_b_0"], (1, 1), (0, 0))
    shark = Conv(relu, weights["fire4/expand1x1_w_0"], weights["fire4/expand1x1_b_0"], (1, 1), (0, 0))
    dogfish = Conv(relu, weights["fire4/squeeze1x1_w_0"], weights["fire4/squeeze1x1_b_0"], (1, 1), (0, 0))
    walrus = Conv(relu, weights["fire3/expand1x1_w_0"], weights["fire3/expand1x1_b_0"], (1, 1), (0, 0))
    frog = Conv(relu, weights["fire3/squeeze1x1_w_0"], weights["fire3/squeeze1x1_b_0"], (1, 1), (0, 0))
    finch = Conv(relu, weights["fire2/expand1x1_w_0"], weights["fire2/expand1x1_b_0"], (1, 1), (0, 0))
    cat = Conv(relu, weights["fire2/squeeze1x1_w_0"], weights["fire2/squeeze1x1_b_0"], (1, 1), (0, 0))
    fly = Conv(relu, weights["conv1_w_0"], weights["conv1_b_0"], (2, 2), (0, 0))
    armadillo = Conv(relu, weights["fire2/expand3x3_w_0"], weights["fire2/expand3x3_b_0"], (1, 1), (1, 1))
    chimpanzee = Conv(relu, weights["fire3/expand3x3_w_0"], weights["fire3/expand3x3_b_0"], (1, 1), (1, 1))
    sandpiper = Conv(relu, weights["fire4/expand3x3_w_0"], weights["fire4/expand3x3_b_0"], (1, 1), (1, 1))
    caterpillar = Conv(relu, weights["fire5/expand3x3_w_0"], weights["fire5/expand3x3_b_0"], (1, 1), (1, 1))
    rhinoceros = Conv(relu, weights["fire6/expand3x3_w_0"], weights["fire6/expand3x3_b_0"], (1, 1), (1, 1))
    reindeer = Conv(relu, weights["fire7/expand3x3_w_0"], weights["fire7/expand3x3_b_0"], (1, 1), (1, 1))
    gaur = Conv(relu, weights["fire8/expand3x3_w_0"], weights["fire8/expand3x3_b_0"], (1, 1), (1, 1))
    dragonfly = Conv(relu, weights["fire9/expand3x3_w_0"], weights["fire9/expand3x3_b_0"], (1, 1), (1, 1))
    (kudu,)->begin 
            waterbuffalo = cat(maxpool(fly(kudu), (3, 3), (0, 0), (2, 2)))
            bear = frog(cat(3, finch(waterbuffalo), armadillo(waterbuffalo)))
            elk = dogfish(maxpool(cat(3, walrus(bear), chimpanzee(bear)), (3, 3), (0, 0), (2, 2)))
            mongoose = koala(cat(3, shark(elk), sandpiper(elk)))
            lemur = rat(maxpool(cat(3, pheasant(mongoose), caterpillar(mongoose)), (3, 3), (0, 0), (2, 2)))
            eland = butterfly(cat(3, eagle(lemur), rhinoceros(lemur)))
            rook = parrot(cat(3, bear(eland), reindeer(eland)))
            ant = kangaroo(cat(3, narwhal(rook), gaur(rook)))
            sealion(cat(3, vicuña(ant), dragonfly(ant)))
        end
end

As you can see, this code is wrong. (Notice the working of bear). However, this is solved if we use unique ids.

[MikeInnes]

What's your version/commit of macrotools there?

[ayush1999]

Cloned the latest version today for testing purposes.

[MikeInnes]

Ok, can you send a version that breaks but printed without alias_gensyms?

[ayush1999]

Generated the file again, but without alias_gensyms. Note that this error doesn't occur every time.
begin

    ##c#672 = Chain(Dropout(0.5f0), Conv(relu, weights["conv10_w_0"], weights["conv10_b_0"], (1, 1), (0, 0)), ((##x#667,)->mean(##x#667, (1, 2))), softmax)
    ##c#673 = Conv(relu, weights["fire9/expand1x1_w_0"], weights["fire9/expand1x1_b_0"], (1, 1), (0, 0))
    ##c#674 = Conv(relu, weights["fire9/squeeze1x1_w_0"], weights["fire9/squeeze1x1_b_0"], (1, 1), (0, 0))
    ##c#675 = Conv(relu, weights["fire8/expand1x1_w_0"], weights["fire8/expand1x1_b_0"], (1, 1), (0, 0))
    ##c#676 = Conv(relu, weights["fire8/squeeze1x1_w_0"], weights["fire8/squeeze1x1_b_0"], (1, 1), (0, 0))
    ##c#677 = Conv(relu, weights["fire7/expand1x1_w_0"], weights["fire7/expand1x1_b_0"], (1, 1), (0, 0))
    ##c#678 = Conv(relu, weights["fire7/squeeze1x1_w_0"], weights["fire7/squeeze1x1_b_0"], (1, 1), (0, 0))
    ##c#679 = Conv(relu, weights["fire6/expand1x1_w_0"], weights["fire6/expand1x1_b_0"], (1, 1), (0, 0))
    ##c#680 = Conv(relu, weights["fire6/squeeze1x1_w_0"], weights["fire6/squeeze1x1_b_0"], (1, 1), (0, 0))
    ##c#681 = Conv(relu, weights["fire5/expand1x1_w_0"], weights["fire5/expand1x1_b_0"], (1, 1), (0, 0))
    ##c#682 = Conv(relu, weights["fire5/squeeze1x1_w_0"], weights["fire5/squeeze1x1_b_0"], (1, 1), (0, 0))
    ##c#683 = Conv(relu, weights["fire4/expand1x1_w_0"], weights["fire4/expand1x1_b_0"], (1, 1), (0, 0))
    ##c#684 = Conv(relu, weights["fire4/squeeze1x1_w_0"], weights["fire4/squeeze1x1_b_0"], (1, 1), (0, 0))
    ##c#685 = Conv(relu, weights["fire3/expand1x1_w_0"], weights["fire3/expand1x1_b_0"], (1, 1), (0, 0))
    ##c#686 = Conv(relu, weights["fire3/squeeze1x1_w_0"], weights["fire3/squeeze1x1_b_0"], (1, 1), (0, 0))
    ##c#687 = Conv(relu, weights["fire2/expand1x1_w_0"], weights["fire2/expand1x1_b_0"], (1, 1), (0, 0))
    ##c#688 = Conv(relu, weights["fire2/squeeze1x1_w_0"], weights["fire2/squeeze1x1_b_0"], (1, 1), (0, 0))
    ##c#689 = Conv(relu, weights["conv1_w_0"], weights["conv1_b_0"], (2, 2), (0, 0))
    ##c#690 = Conv(relu, weights["fire2/expand3x3_w_0"], weights["fire2/expand3x3_b_0"], (1, 1), (1, 1))
    ##c#691 = Conv(relu, weights["fire3/expand3x3_w_0"], weights["fire3/expand3x3_b_0"], (1, 1), (1, 1))
    ##c#692 = Conv(relu, weights["fire4/expand3x3_w_0"], weights["fire4/expand3x3_b_0"], (1, 1), (1, 1))
    ##c#693 = Conv(relu, weights["fire5/expand3x3_w_0"], weights["fire5/expand3x3_b_0"], (1, 1), (1, 1))
    ##c#694 = Conv(relu, weights["fire6/expand3x3_w_0"], weights["fire6/expand3x3_b_0"], (1, 1), (1, 1))
    ##c#695 = Conv(relu, weights["fire7/expand3x3_w_0"], weights["fire7/expand3x3_b_0"], (1, 1), (1, 1))
    ##c#696 = Conv(relu, weights["fire8/expand3x3_w_0"], weights["fire8/expand3x3_b_0"], (1, 1), (1, 1))
    ##c#697 = Conv(relu, weights["fire9/expand3x3_w_0"], weights["fire9/expand3x3_b_0"], (1, 1), (1, 1))
    (##x#698,)->begin 
            ##edge#699 = ##c#688(maxpool(##c#689(##x#698), (3, 3), (0, 0), (2, 2)))
            ##edge#700 = ##c#686(cat(3, ##c#687(##edge#699), ##c#690(##edge#699)))
            ##edge#701 = ##c#684(maxpool(cat(3, ##c#685(##edge#700), ##c#691(##edge#700)), (3, 3), (0, 0), (2, 2)))
            ##edge#702 = ##c#682(cat(3, ##c#683(##edge#701), ##c#692(##edge#701)))
            ##edge#703 = ##c#680(maxpool(cat(3, ##c#681(##edge#702), ##c#693(##edge#702)), (3, 3), (0, 0), (2, 2)))
            ##edge#704 = ##c#678(cat(3, ##c#679(##edge#703), ##c#694(##edge#703)))
            ##edge#705 = ##c#676(cat(3, ##c#677(##edge#704), ##c#695(##edge#704)))
            ##edge#706 = ##c#674(cat(3, ##c#675(##edge#705), ##c#696(##edge#705)))
            ##c#672(cat(3, ##c#673(##edge#706), ##c#697(##edge#706)))
        end
end

[philtomson]

One problem I see is that you end up with two 'bears' in your list of animal symbols because animals.txt has entries for both "polar bear" and "bear". In the init function in src/MacroTools.jl the "polar bear" entry gets split into two separate entries (in this code):

function __init__()
  animals_file = joinpath(dirname(@__FILE__), "..", "animals.txt")
  _animals = split(read(animals_file, String))
  resize!(animals, length(_animals))
  animals .= Symbol.(lowercase.(_animals))
  Compat.Random.shuffle!(animals)
end

There is already a separate bear entry in animals.txt so you end up with duplicate bears.

I tried it out in the REPL (prior to the shuffle):

julia> animals[160:165]
6-element Array{Symbol,1}:
 :pigeon   
 :polar    
 :bear     
 :pony     
 :porcupine
 :porpoise 

So 'polar' and 'bear' were split into separate entries, but there was also the previous bear entry:

julia> find(x -> x == :bear, animals)
2-element Array{Int64,1}:
  15
 162

The same problem exists for "seal" because there are both "elepant seal" and "seal" entries in animals.txt:

julia> find(x -> x == :seal, animals)
2-element Array{Int64,1}:
  64
 188

(there are also "panda" and "giant panda" entries in animals.txt, so "panda" is a duplicate as well.

The random shuffle explains the intermitant nature of this problem. Probably best to eliminate spaces in the names in animals.txt.

[MikeInnes]

Ah, great catch, thanks. I'll fix that up and get this merged.

[ayush1999]

@MikeInnes I think there should be an option to use generated tokens, rather than animal names. This will solve two problems:

1. In case the numbers of names needed is more than the names present.
2. Easier to debug.

Any thoughts?

[MikeInnes]

Isn't that what I just merged?