Closed 51N84D closed 4 years ago
good wip :)
updating tests is good writing new one is best :p (not immediately, but before merging)
Ok, I added back the discriminators. Is it ready to merge into master?
@51N84D I think it's ready enough for Tianyu and I to start working on it!
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
inm
That would be useful! instead of having to comment the discriminators in and out
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
inm
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
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
inm
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 ?
what do you mean ignore default? what problem do you have?
@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
@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!
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.