Skip to content

Conversation

@DavdGao
Copy link
Collaborator

@DavdGao DavdGao commented Sep 21, 2022

Our Target

  • We are planning to add a consultation stage before training for differential privacy.
  • We decompose it into several PRs to avoid large modification at one time.

What's in this PR

  • This PR is to support more message types in the server (currently we only support train and eval in self.msgbuffer)
  • We add a enumeration class STAGE in enums.py to avoid using string directly
  • We modify check_and_move_on and check_buffer fucntions to support more messages.
  • The modification is compatible with previous versions

@DavdGao DavdGao added the Feature New feature label Sep 21, 2022
@DavdGao DavdGao requested review from joneswong and removed request for joneswong September 21, 2022 08:56
@DavdGao DavdGao changed the title [Feature] Support more message type in server [Feature] Support more message types in server Sep 21, 2022
state, content, timestamp = message.state, message.content, \
message.timestamp
self.msg_buffer['train'][state].append(content)
self.msg_buffer[STAGE.TRAIN][state].append(content)
Copy link
Collaborator

@rayrayraykk rayrayraykk Oct 11, 2022

Choose a reason for hiding this comment

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

I'm not sure whether the 'stage' is an appropriate key name, as the key of msg_buffer should be message type (although it is not now: msg_buffer['train'] contains model, msg_buffer['eval'] contains eval results).

Maybe we should use the message type as the key of msg_buffer (like msg_buffer['model_param'] ).

What's more, are STAGE and state confusing?

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.

Please see the inline comments, and the other part looks good to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature New feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants