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

Why is reweight_groups flag set for DRO algorithm? Possible unfair comparison to ERM? #6

Closed
lokhande-vishnu opened this issue Aug 3, 2021 · 1 comment

Comments

@lokhande-vishnu
Copy link

lokhande-vishnu commented Aug 3, 2021

Dear authors, Thank you for sharing a well polished codebase!

For the results in table 1, I have noticed that DRO method is always run with "reweight_groups" flag set to "True", whereas the same flag is "False" for the ERM algorithm [1]. As per the code, the "reweight_groups" flag performs a weighted random sampling guaranteeing an equal count of each group in any given batch. On the other hand, the ERM algorithm receives a smaller count of the minority sample as there is no weighted random sampling. Such a difference in implementation between ERM and GroupDRO suggests for an unfair comparison between the two methods.

Surely, as pointed out in the comment [2], the loss function could be considered unaffected by the "reweight_groups" flag as the DRO method uses the mean of per-group losses. However, the empirical estimate of these means in a given batch would be highly noisy when the sample count of the minority group is very small. This makes me wonder (and I hope it's okay for me to ask), that the gains reported in the paper are attributed solely to the use of weighted random sampling procedure rather than DRO update rule? Please clarify

Do you have any comparisons of the DRO algorithm with "reweight_groups" flag set to "False"? How does ERM with "reweight_flag=True" compare to ERM with "reweight_flag=False"?

Thank you

[1] https://worksheets.codalab.org/worksheets/0x621811fe446b49bb818293bae2ef88c0
[2]

# since the minibatch is only used for mean gradient estimation for each group separately

@kohpangwei
Copy link
Owner

Hi Vishnu,

Yes, we do compare ERM with reweight_groups=True. This is actually the main empirical comparison we do in the paper. See the "Empirical comparison" paragraph under Section 4 in our paper: https://arxiv.org/pdf/1911.08731.pdf

We didn't try DRO without reweighting the groups since, as you mention, this would make the per-group estimates more noisy.

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

No branches or pull requests

2 participants