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

Minor fixes for GitHub workflow action and the mean/std values of DataTransforms #513

Merged
merged 4 commits into from
Feb 10, 2023

Conversation

xieyxclack
Copy link
Collaborator

This pr includes two minor fixes:

  • Change the rule for triggering the GitHub workflow action test_act: from pull_request: types: [opened, synchronize, edited] to schedule: cron: '0 8 * * 0'. It implies that test_act would no longer be triggered when someone gives a pull request, it would only be triggered once a week. The reasons for this modification are:
    • It needs lots of time for running test_act and causes the unnecessary resource cost;
    • test_atc is just a unit-test for one specific FL-NLP application but not a general functionality, therefore most of the developments would be changed its correctness.
    • Running test_atc requires more packages such as Rouge and METEOR. Considering that these packages are only used in NLP applications, we don't include these packages in the environment. Thus we perform this unit-test via GitHub workflow action (which is easier to include the package installation during testing) rather than running it via Jenkins. Later test_atc can be moved to our server when these requirements have been installed.
  • Change the mean/std values for DataTransforms
    • For FEMNIST: mean=[0.9637], std=[0.1592]. These values are calculated by ourselves and are similar to those provided by other researchers.
    • For CIFAR10: mean= [0.4914, 0.4822, 0.4465], std= [0.2470, 0.2435, 0.2616], which is borrowed from the value of mean, std in cifar-10  facebookarchive/fb.resnet.torch#180
    • For other cv datasets containing 3-channel figures: mean= [0.485, 0.456, 0.406], std= [0.229, 0.224, 0.225], which are standard values calculated from ImageNet

We point out that when BatchNorm is adopted, the mean/std values of DataTransforms make little effect on the model performance. And we haven't changed the yaml files of benchmarking (those under FederatedScope/benchmark/).

@xieyxclack xieyxclack added the enhancement New feature or request label Feb 7, 2023
rayrayraykk
rayrayraykk previously approved these changes Feb 7, 2023
Copy link
Collaborator

@rayrayraykk rayrayraykk left a comment

Choose a reason for hiding this comment

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

LGTM, @DavdGao, please double-check the mean/std value.

@rayrayraykk rayrayraykk requested a review from DavdGao February 7, 2023 09:27
Copy link
Collaborator

@DavdGao DavdGao left a comment

Choose a reason for hiding this comment

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

LGTM

@xieyxclack xieyxclack merged commit 1ecd4a6 into alibaba:master Feb 10, 2023
@xieyxclack xieyxclack deleted the bugfix_yaml branch February 10, 2023 02:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants