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

Add empirical sampling mode to the RCF model #1339

Merged
merged 12 commits into from
Sep 29, 2023
Merged

Add empirical sampling mode to the RCF model #1339

merged 12 commits into from
Sep 29, 2023

Conversation

calebrob6
Copy link
Member

@calebrob6 calebrob6 commented May 15, 2023

The MOSAIKS paper describes RCF, which we've already implemented, and this method (which I'll call MOSAIKS). This method is the same setup as RCF but instead of initializing the conv kernels randomly, you sample patches from the dataset, do ZCA whitening, and use those as the kernels. In some little experiments on EuroSAT and UCM this gives +2 to 5% accuracy over simple RCF kernels.

@github-actions github-actions bot added the models Models and pretrained weights label May 15, 2023
isaaccorley
isaaccorley previously approved these changes May 15, 2023
Copy link
Collaborator

@isaaccorley isaaccorley left a comment

Choose a reason for hiding this comment

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

Just need to add to docs

@calebrob6
Copy link
Member Author

Lol need to figure out some type hints and stuff too

@github-actions github-actions bot added the documentation Improvements or additions to documentation label May 15, 2023
@calebrob6
Copy link
Member Author

probably tests too huh?

@isaaccorley
Copy link
Collaborator

Nah who needs tests???

@github-actions github-actions bot added the testing Continuous integration testing label May 15, 2023
@calebrob6
Copy link
Member Author

Okay nowwww it LGTM

@adamjstewart adamjstewart added this to the 0.5.0 milestone May 16, 2023
@adamjstewart
Copy link
Collaborator

Will review post paper

@estherrolf
Copy link
Contributor

estherrolf commented May 16, 2023

chiming in at Caleb's request on the naming of the option with Gaussian patches and the option with patches drawn from the dataset:

I would call both of these random convolutional features, but in the first the random patches are drawn from a Gaussian distribution and in the second from the empirical distribution of image patches. Thus, I would propose "RCF - Gaussian" and "RCF - Empirical" to distinguish between them, noting that the latter (RCF - empirical) is what what we use in the MOSAIKS paper.

To me the most straightforward would be to have one RCF class, with argument for patch_distribution, where that would be either empirical or Gaussian. It's possible that would be harder to implement, so if it's better to have two different classes, maybe RCF_empirical and RCF_gaussian?

LMK if I can help clarify further!

@isaaccorley
Copy link
Collaborator

Would RCF-ZCA or RCF-Empirical-ZCA be better since it's more explicit? I'm assuming we could probably use other techniques than ZCA to estimate the kernels using the dataset.

@estherrolf
Copy link
Contributor

Yeah using ZCA to whiten the images is an optional preprocessing step but we've found that it can help quite a bit. I still think the most straightforward would be to have one RCF class with arguments for patch_distribution as either "gaussian" or "empirical" and have the ZCA whitening step as an optional (but default?) processing step. Alternatively having two classes (RCF_Gaussian) and (RCF_empirical) and for both of them having ZCA whitening step as an argument make sense.

@calebrob6 calebrob6 closed this Jun 20, 2023
@calebrob6 calebrob6 reopened this Jun 20, 2023
Copy link
Collaborator

@adamjstewart adamjstewart left a comment

Choose a reason for hiding this comment

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

Since we added RCF first, all files and documentation are named after RCF. I haven't read the paper, but my only question is whether we should rename things to be under the RCF or MOSAIKS umbrella.

torchgeo/models/rcf.py Outdated Show resolved Hide resolved
torchgeo/models/rcf.py Show resolved Hide resolved
torchgeo/models/rcf.py Outdated Show resolved Hide resolved
torchgeo/models/rcf.py Outdated Show resolved Hide resolved
torchgeo/models/rcf.py Outdated Show resolved Hide resolved
@adamjstewart adamjstewart removed this from the 0.5.0 milestone Sep 28, 2023
@github-actions github-actions bot removed the documentation Improvements or additions to documentation label Sep 28, 2023
@calebrob6
Copy link
Member Author

@adamjstewart @estherrolf -- I combined the functionality of both under a single RCF class that takes mode="empirical" or mode="gaussian". I actually like this way better than having two classes and, as usual, am thankful we do thorough peer review :). In particular, I can imagine us adding a few more parameters to this class in the future to allow for easy experimentation over dimensions like "normalization schemes". Having the code in one class is better for this.

@calebrob6 calebrob6 changed the title Add MOSAIKS model Add empirical sampling mode to the RCF model Sep 28, 2023
Copy link
Collaborator

@adamjstewart adamjstewart left a comment

Choose a reason for hiding this comment

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

I'm fine with the new organization

torchgeo/models/rcf.py Show resolved Hide resolved
torchgeo/models/rcf.py Outdated Show resolved Hide resolved
@adamjstewart adamjstewart added this to the 0.5.0 milestone Sep 28, 2023
torchgeo/models/rcf.py Outdated Show resolved Hide resolved
@adamjstewart adamjstewart enabled auto-merge (squash) September 29, 2023 10:29
@adamjstewart adamjstewart merged commit 51ffb69 into main Sep 29, 2023
21 checks passed
@adamjstewart adamjstewart deleted the mosaiks branch September 29, 2023 13:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
models Models and pretrained weights testing Continuous integration testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants