[WIP] From 4 to 2 domains, and adding masker functionality

[51N84D]

Instead of four domains {real, sim}_{flooded, nonflooded} we now only have two {real, sim}. I've also added a "Masker" decoder. I've retained the "Water" decoder because we may want to use that for the auxiliary task of water segmentation (which is different from mask generation).

Also, all of the tests should pass but I haven't tried running a real training experiment.

[vict0rsch]

good wip :)

updating tests is good writing new one is best :p (not immediately, but before merging)

[51N84D]

Ok, I added back the discriminators. Is it ready to merge into master?

[melisandeteng]

@51N84D I think it's ready enough for Tianyu and I to start working on it!

[melisandeteng]

Works fine ! Great work!
(of course the discriminators part should be left out for the moment when trying to run an experiment though)

[vict0rsch]

question before we merge: do we consider discriminator(s) for the masked to be part of the advent PR or should that already be a flag here? it would ~ boil down to adding a key in the OmniDiscriminator class and a key like generative:True|False in m

[melisandeteng]

question before we merge: do we consider discriminator(s) for the masked to be part of the advent PR or should that already be a flag here? it would ~ boil down to adding a key in the OmniDiscriminator class and a key like generative:True|False in m

That would be useful! instead of having to comment the discriminators in and out

[51N84D]

question before we merge: do we consider discriminator(s) for the masked to be part of the advent PR or should that already be a flag here? it would ~ boil down to adding a key in the OmniDiscriminator class and a key like generative:True|False in m

That would be useful! instead of having to comment the discriminators in and out

Right now the code is setup such that if the "OmniDiscriminator" is empty (as in no parameters) it gets ignored. I think we can wait to add the ADVENT key with the ADVENT PR

[melisandeteng]

question before we merge: do we consider discriminator(s) for the masked to be part of the advent PR or should that already be a flag here? it would ~ boil down to adding a key in the OmniDiscriminator class and a key like generative:True|False in m

That would be useful! instead of having to comment the discriminators in and out

Right now the code is setup such that if the "OmniDiscriminator" is empty (as in no parameters) it gets ignored. I think we can wait to add the ADVENT key with the ADVENT PR

Oh ok I hadn't noticed that. I'm wondering is there a way to ignore default settings then ?

[vict0rsch]

what do you mean ignore default? what problem do you have?

[51N84D]

@vict0rsch actually, I may have made the commit that un-comments the discriminator stuff after Méli already cloned the branch...
@melisandeteng can you try pulling those changes and running again? You shouldn't have to comment anything out

[melisandeteng]

@vict0rsch actually, I may have made the commit that un-comments the discriminator stuff after Méli already cloned the branch...
@melisandeteng can you try pulling those changes and running again? You shouldn't have to comment anything out

It works perfectly fine! for some reason when I first had pulled the uncommented version it would output an error message with "Key "a" does not exist" in the tasks so I thought it was fetching the default parameters but somehow I don't have this issue anymore!