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

Memory leak Details #1155

Closed
wants to merge 7 commits into from
Closed

Conversation

payalcha
Copy link
Contributor

No description provided.

Signed-off-by: Chaurasiya, Payal <[email protected]>
@payalcha payalcha changed the title Initial commit Memory leak Details Nov 18, 2024
Ubuntu added 2 commits November 19, 2024 07:27
Signed-off-by: Ubuntu <azureuser@vm-openfl.edrucphba2zutobdsbafmzy3ya.bx.internal.cloudapp.net>
Signed-off-by: Ubuntu <azureuser@vm-openfl.edrucphba2zutobdsbafmzy3ya.bx.internal.cloudapp.net>
openfl/component/aggregator/aggregator.py Outdated Show resolved Hide resolved
@@ -132,6 +132,8 @@ def modify_plan(self, new_rounds=None, num_collaborators=None, disable_client_au
data = yaml.load(fp, Loader=yaml.FullLoader)

data["aggregator"]["settings"]["rounds_to_train"] = int(self.rounds_to_train)
data["aggregator"]["settings"]["memleak_check"] = True
Copy link
Collaborator

Choose a reason for hiding this comment

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

in my opinion, we should enable this by default.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Memory details are not needed for all runs. Basically it will be better for performance and Memory Leak related tests I think.
Please let me know if still needs to change this.

openfl/component/collaborator/collaborator.py Show resolved Hide resolved
Signed-off-by: payalcha <[email protected]>
Copy link
Collaborator

@teoparvanov teoparvanov left a comment

Choose a reason for hiding this comment

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

A couple of comments, as per our offline discussion

openfl-workspace/torch_cnn_mnist/plan/plan.yaml Outdated Show resolved Hide resolved
openfl/component/collaborator/collaborator.py Show resolved Hide resolved
openfl/component/aggregator/aggregator.py Show resolved Hide resolved
tanwarsh and others added 2 commits November 19, 2024 13:35
* deprecate python native api

Signed-off-by: yes <[email protected]>

* syntax issue fix

Signed-off-by: yes <[email protected]>

---------

Signed-off-by: yes <[email protected]>
Copy link
Collaborator

@teoparvanov teoparvanov left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @payalcha !

@@ -171,6 +174,24 @@ def run(self):

# Cleaning tensor db
self.tensor_db.clean_up(self.db_store_rounds)
if self.log_memory_usage:
# This is the place to check the memory usage of the collaborator
self.logger.info("*****************COLLABORATOR LOGS*******************************")
Copy link
Collaborator

Choose a reason for hiding this comment

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

********* COLLABORATOR MEMORY USAGE *********?

@payalcha payalcha mentioned this pull request Nov 20, 2024
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

Successfully merging this pull request may close these issues.

4 participants