Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Abstractions #38

Merged
merged 117 commits into from
Aug 13, 2020
Merged

Abstractions #38

merged 117 commits into from
Aug 13, 2020

Conversation

billbrod
Copy link
Member

@billbrod billbrod commented Apr 10, 2020

This pull requests address #26 and #17: it adds MAD competition and an abstract Synthesis superclass. As part of this, it reworks Metamer to move a lot of shared code to Synthesis, and Metamer and MAD competition now both inherit from this class.

This is not ready to be merged yet, but it's very large and so wanted to put it up here to give folks some time to look at it. The content is basically all done, but I'm adding some tutorials before I'd like to merge it.

Right now, I've started a tutorial on how to use MAD Competition (check it out at examples/MAD_Competition.ipynb), and I'd like to finish that as well as tutorials on Metamer (which is overdue) and the Synthesis superclass. There will be some shared content across them (that was the goal of writing the superclass, that usage and call signatures would be similar).

This also adds a load_images function (in tools/data.py) that hopefully loads in images the way we want to: making sure it's the right dtype, number of dimensions, handling 8 or 16 bit intelligently, converting to grayscale if wanted. It accepts a single path or list of paths (as long as they're all the same size). Could also probably modify it to work on arrays (or add another image that does that), but the advantage of working from paths is we know we can figure out what data type it was stored as (8 or 16 bit integers).

helper function for loading in images and making sure they're the
right shape

with tests
moves a bunch of code from Metamer to Synthesis. currently not much
change to those methods, but will do so as attempt to use the
abstraction for geodesics, eigendistortions, and MAD
with this commit, we add an initial implementation of MAD
competition. it currently only minimizes model 1 while holding model 2
constant, and only works on models (not metrics), but the basic
framework is there. a fair amount of cleanup is required as well.

MAD competition (like Metamer) inherits the Synthesis class and so
makes use of much there. more functionality consolidation is needed
here, as there was a lot of copy-and-paste

 - Synthesis now uses the self.names dictionary to determine what to
   target / match etc. this doesn't change how Metamer works (or
   honestly, probably any of the other synthesis methods) but is
   necessary for MAD

 - adds a base gradient descent optimizer choice, GD

 - scheduler is now optional

 - optimizer_step's pbar arg is now optional. if None, will not update
   information. can also add additional information to the pbar as
   kwargs
originally, I was planning on adding an arg to objective_function to
turn normalizing on/off, but now we always do it if possible
with this, all targets are now supported, which requires an overhaul
of how we handle getting and setting the relevant attributes.

Synthesis goes back to knowing nothing about the attribute structure
of its children classes, assuming it can just directly grab
self.matched_representation (instead of going through self.names). it
is the responsibility of the child class to make sure this is the
correct attribute to modify, which we do here by modifying the getter
and setter (and continued use of a self._names attribute)

 - adds way more documentation to mad competition

 - moves representation_error to Synthesis

 - mad competition has own representation_error method, to handle the
   multiple synthesis targets / models

 - normalizing the loss is now an option, not always done

 - many MADCompetition attributes now have "all" variants, which store
   across targets
it's now in Synthesis, mad_competition has its own version (like
representation_error)
this moves the following functions from the Metamer class to
Synthesis, so it can be used by other synthesis methods:
plot_representation_error, plot_synthesis_status (was
plot_metamer_status), animate

other changes, to MADCompetition:

 - corrects name from saved_representation_gradient_{num} to
   saved_representation_{num}_gradient

 - now correctly handles _all attributes and copying information back
   and forth between them and the local version, does so with helper
   functions

 - nu, learning_rate, gradient now lists with _all versions that are
   dicts (like the other attributes)

 - adds _check_state() helper function which updates the synthesis
   target if necessary and returns the current state (used by the
   various plotting functions)
variety of updates necessary to make this happen:

 - mad competition has wrappers around many of the functions, which
   just make sure that the synthesis target is appropriately set. we
   make sure these have the args in the same order as the Synthesis
   class's version (with the new args tucked on the end)

 - mad_competition.representation_error's model arg can also be
   'both', which returns a dictionary containing both model's
   representation errors

 - correctly handle the copying attributes in *and out* of _all for
   tensors and lists

 - adds methods for MADCompetition.plot_synthesized_image,
   plot_synthesized_image_all (shows all synthesized images),
   plot_loss, plot_loss_all, plot_synthesis_status, and animate

 - in Synthesis, split out plot_synthesized_image and plot_loss from
   plot_synthesis_status

 - Synthesis.animate now takes two more args, plot_data_attr, a list
   which specifes the attributes to plot on the second subplot (so can
   be more than one; thus we grab all artists), and rep_error_kwargs,
   which specifies additional args to pass to the
   self.representation_error() call

 - display.rescale_ylim works with an array like or a dict. if a
   dictionary, we take the max over all of its values
for mad competition, can set whether you want to normalize the two
models' losses or keep them the same. unsure which is best right now
because it is likely to have positive and negative values
the MAD competition paper, as originally written, compared SSIM and
MSE, which are metrics, not models (they take two images and return a
scalar distance between them). as our code was written, it only worked
for models (which take an image and return a representation, and we
use the L2-norm of the difference between representations for the
distance between images).

with this commit, can now use metrics for MAD competition. to do that,
we construct a 'dummy model', that just returns a copy of the image
and modify the function called for our objective function

other changes

 - adds Identity() class in simulate/models/naive.py

 - adds MSE class to mse() function, since metrics should be functions
   now

 - we no longer do the whole randomizing of pixels before computing
   the loss in Synthesis._closure(). this really messes up those
   metrics that extract something meaningful from the image (i.e., all
   the ones we're interested in). so this is turned off by default,
   and only turned on when needed. right now, this is just when
   metamer has fraction_removed > 1 or loss_change_fraction <
   1. should try and generalize that tools

 - updates docstrings

 - adds loss_function attr, which is how we switch things around for
   the two models. objective_function calls this on x and y. (still
   use objective_function because it handles the normalizing)

 - adds _get_model_name, which first tries to grab the model's name
   attribute and, if it doesn't have one, then returns the class
   name. useful for the Identity class

 - raise a warning whenever the representation error is computed with
   a metric, since it will be meaningless
 - renames MSE to mse in import statement

 - replaces plot_metamer_status with plot_synthesis_status
realized there was more stuff that can be done by the superclass
two big changes, for Synthesis, MADCompetition, Metamer:

 - metric support added. this just moves the bits that handle this
   from mad_competition.py to Synthesis.py

 - can set loss_function to custom function. by default is L2-norm of
   difference

 - updates everyone's documentation and call signatures
this commit breaks the components of synthesize() into functions,
moves them to Synthesize abstract metaclass as possible, and thus
makes things more readable

as part of doing this, realized why matched_representation was
sometimes a Parameter after saving: because I was making it one when a
NaN was hit during loss. removed that and everything seems to still
work

still need to:

 - figure out how I want to handle coarse-to-fine and the randomizing
   stuff

 - standardize synthesize() call signature
"source": [
"# Implementing New Synthesis Methods\n",
"\n",
"This notebook assumes you are familiar with synthesis methods and have interacted with them in this package, and would like to either understand what happens under the hood a little better or implement your own using the existing API.\n",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we have an executable example for this ipynb (even if really dumb/simple) ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I couldn't come up with what an example would look like here. Any thoughts?

Otherwise, this should just probably not be a notebook, and make it a rst in the docs folder instead.

Copy link
Member

@lyndond lyndond left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

examples/simple_example.ipynb
I like the idea of this. Perhaps later on we can throw in an example of each synthesis method using the same model(s) and image.

But also: Do models not also need a valid backward method for most our synthesis procedures?

Copy link
Member

@lyndond lyndond left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

examples/Metamer.ipynb
I like it but think it could benefit from more headers/subheaders to break it up a bit.

Copy link
Member

@lyndond lyndond left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

examples/MAD_Competition.ipynb

I found it hard to understand what exactly the plots mean when I'm looking at them. Could perhaps in more detail say what's going on in each of the subplots after the 5th cell, containing:

mad.synthesize('model_1_min', store_progress=True, max_iter=200)
fig = mad.plot_synthesis_status()

Some thoughts I think readers may have:

  1. Are the two right subplots representation error? Or is the lineplot loss the representation error.
  2. Why do you say the loss decreases but then both of them change direction and converge to the same value?
  3. You outline the four cases in the intro, but can you give me some intuition on what I should expect to see in the loss plots before you show the first one to me?
  4. Some subheaders to break up the sections would be nice.

@lyndond
Copy link
Member

lyndond commented Aug 11, 2020

I believe that in docstrings, input parameters should be expecting torch.Tensor and not torch.tensor; the former is an object & the latter is a function that returns a torch.Tensor. My docstring renderer in pycharm recognizes torch.Tensor and formats the docstring nicely with it but not when I've used the lowercase version.

https://stackoverflow.com/questions/51911749/what-is-the-difference-between-torch-tensor-and-torch-tensor

plenoptic/tools/data.py Outdated Show resolved Hide resolved
plenoptic/tools/data.py Outdated Show resolved Hide resolved
plenoptic/tools/data.py Outdated Show resolved Hide resolved
base_signal : torch.tensor or array_like
A 4d tensor, this is the image whose representation we wish to
match. If this is not a tensor, we try to cast it as one.
model_1, model_2 : torch.nn.Module or function
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a thought: I think my relative eigendist code (synth distortions to maximize response in one layer while minimizing response in a subsequent layer) can use a lot of these same attrs and params.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent!

Copy link
Member

@lyndond lyndond left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly minor things in docstrings, and some clarity/arrangement issues w/ ipynbs. I'd be ok w/ you just fixing docstrings and merging, then coming back to ipynbs at a later time.

@billbrod
Copy link
Member Author

plenoptic/metric/naive.py
What's the reason for this method to be all alone? Do we expect to add more?

I thought we might want to add more naive metrics (just compare means, locally or globally, or other moments). Also, I didn't think it made sense to put this in one of the existing files.

@billbrod
Copy link
Member Author

examples/simple_example.ipynb
I like the idea of this. Perhaps later on we can throw in an example of each synthesis method using the same model(s) and image.

But also: Do models not also need a valid backward method for most our synthesis procedures?

Yeah, I think being able to plot in image space makes it a little simpler to see what the different synthesis methods are doing. It would be cool to have them for all our methods.

All models need a backwards method, but as long as you create your model out of the existing torch machinery, it will automatically define it for you. So, in practice, users don't need to worry about it.

@billbrod
Copy link
Member Author

examples/Metamer.ipynb

I like it but think it could benefit from more headers/subheaders to break it up a bit.

examples/MAD_Competition.ipynb

I found it hard to understand what exactly the plots mean when I'm looking at them. Could perhaps in more detail say what's going on in each of the subplots after the 5th cell, containing:

mad.synthesize('model_1_min', store_progress=True, max_iter=200)
fig = mad.plot_synthesis_status()

Some thoughts I think readers may have:

1. Are the two right subplots representation error? Or is the lineplot loss the representation error.

2. Why do you say the loss decreases but then both of them change direction and converge to the same value?

3. You outline the four cases in the intro, but can you give me some intuition on what I should expect to see in the loss plots before you show the first one to me?

4. Some subheaders to break up the sections would be nice.

I agree that these two need more work before they're really understandable. I'd like to come back to that later, because I think it will require more work. Also I reran the MAD_Competition notebook and re-generated its figures, because I realized I had changed how the initial noise was being created and it led to worse examples (which didn't match the text), so hopefully it looks better now.

- bugfix in non_linearities.py, need to use :meth: tag in See Also
  section

- Adds some tutorials and example notebooks

- updates the auto-generated rst files

- changes some of the markdown headings of Eigendistortions and
  Original_MAD notebooks
- all tests now import DEVICE, DTYPE, DATA_DIR from test_plenoptic

- makes sure the constants are in caps

- DTYPE is now torch.float32

- removes unnecessary imports
@billbrod billbrod merged commit 649a8c5 into master Aug 13, 2020
@billbrod billbrod deleted the abstractions branch August 13, 2020 17:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants