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

Abstract classes for model and synthesis objects? #17

Closed
billbrod opened this issue Nov 15, 2019 · 5 comments
Closed

Abstract classes for model and synthesis objects? #17

billbrod opened this issue Nov 15, 2019 · 5 comments
Projects

Comments

@billbrod
Copy link
Member

In pyrtools, we had a pyramid class that we never wanted anyone to use, but that all the pyramid objects inherited. This made it easy to share relevant methods between them and make sure they had comparable attributes.

Should we do a similar thing for model and synthesis objects? I've written a whole bunch of code for the ventral stream models and for metamer that I feel could be relevant for other models and synthesis objects, and it would make standardization of the API easier. Using an abstract master class would make it easy to share these methods and make sure the attributes are consistent without requiring too much overhead (once the initial creation of the parent classes is finished...)

I've been meaning to abstract some of the stuff I've written for metamer and ventral stream models regardless. For example, the save and load methods (as well as the "reduced" version for the ventral stream model), and the display code. I've done a bit of work making the display code abstract already, but if we put it in a parent class, you'd have access to it for free.

@billbrod
Copy link
Member Author

billbrod commented Dec 6, 2019

For synthesis, does it make sense to have a __init__ method that gets inherited? All of them expect a model and one (MAD, metamer, eigendistortion) or two (geodesics) images when initialized. They currently have different names in all of the classes: reference_image (MAD), target_image (metamers), image (eigendistortion), and imgA/imgB (geodesics). Should we make them more similar or not?

@billbrod
Copy link
Member Author

billbrod commented Dec 6, 2019

What about objective_function()? Eigendistortion doesn't need one, and it might be different for all the others

@billbrod
Copy link
Member Author

Also make sure to make VentralModel an abstract class

@billbrod billbrod added this to Short term milestones -- in progress in Roadmap Feb 7, 2020
@billbrod billbrod mentioned this issue Feb 7, 2020
4 tasks
@billbrod
Copy link
Member Author

billbrod commented Feb 28, 2020

  • MAD competition has support for metrics, but that can be generalized, so move that to here.

@billbrod billbrod mentioned this issue Apr 10, 2020
@billbrod
Copy link
Member Author

Synthesis added in #38 to serve as a superclass for other synthesis methods. Will open another issue for models when we get there

Roadmap automation moved this from Short term milestones -- in progress to Done Aug 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Roadmap
  
Done
Development

No branches or pull requests

1 participant