cc-ai / climategan

Code and pre-trained model for the algorithm generating visualisations of 3 climate change related events: floods, wildfires and smog.
https://thisclimatedoesnotexist.com
GNU General Public License v3.0
75 stars 18 forks source link

add filter_samples() #48

Closed vict0rsch closed 4 years ago

vict0rsch commented 4 years ago

@51N84D for the sake of the exercise can you write a. test in test_data?

vict0rsch commented 4 years ago

@51N84D for the sake of the exercise can you write a test in test_data?

51N84D commented 4 years ago

@51N84D for the sake of the exercise can you write a test in test_data?

Sure thing

51N84D commented 4 years ago

Also, the filter_samples() function uses the instance variable self.opts but self.opts is never defined. I'll fix that as well

51N84D commented 4 years ago

Oh, and filter_samples() gets rid of the x variable in the sample (since it's technically not a "task" variable) but we probably want to keep that one ;)

vict0rsch commented 4 years ago

write the tests, I'll fix that now :)

vict0rsch commented 4 years ago

@51N84D I think this makes more sense:

    self.tasks = set(opts.tasks)
    self.tasks.add("x")

...

    def filter_samples(self):
        """
        Filter out data which is not required for the model's tasks
        as defined in opts.tasks
        """
        self.samples_paths = [
            {k: v for k, v in s.items() if k in self.tasks}
            for s in self.samples_paths
        ]
vict0rsch commented 4 years ago

samples might not all have the same available data so you can't trust the first sample

51N84D commented 4 years ago

samples might not all have the same available data so you can't trust the first sample

So better would be to iterate through all the samples and check that the variables are subsets of the task variables?

vict0rsch commented 4 years ago

yes, the piece of code I commented above would work don't you agree?

51N84D commented 4 years ago

yes, the piece of code I commented above would work don't you agree?

I agree that it does what we want in the dataset class, but I meant more how do we verify that it's doing what we want in the test program (which is to check that the sample variables are indeed subsets of the task variables)

vict0rsch commented 4 years ago

what about now @51N84D ?

51N84D commented 4 years ago

what about now @51N84D ?

Haha yes, I think we're ready to merge