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

Flag to remove intermediate tasks' states #289

Merged
merged 5 commits into from
Jun 16, 2023
Merged

Flag to remove intermediate tasks' states #289

merged 5 commits into from
Jun 16, 2023

Conversation

prabhuteja12
Copy link
Contributor

Large datasets' buffers can be too big and result in storage issues (noticed while training CLEAR). This flag eliminates backing up intermediate results and overwrites the same folder/files in each update.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@prabhuteja12 prabhuteja12 requested a review from wistuba June 8, 2023 08:20
@@ -179,6 +180,10 @@ def execute_experiment_job(
deterministic_trainer: When true the Trainer adopts a deterministic behaviour also on GPU.
In this function this parameter is set to True by default.
job_name: Name of the experiment job.
strategy: String denoting lightning distributed strategy.
precision: String for which precision to use.
retain_intermediate_state: Flag to retain intermediate models and buffer states.
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe rephrase to "retain states after each update"? otherwise very redundant description and not helpful for anyone who doesn't understand what "intermediate" refers to.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@@ -57,6 +57,7 @@
JOB_NAME = "renate"
SUPPORTED_TUNING_MODE = ["min", "max"]
SUPPORTED_TUNING_MODE_TYPE = Literal["min", "max"]
RETAIN_INTERMEDIATE_STATE = True
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's set it to False.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The current mode of operation is True. We shouldn't alter that, right?

Copy link
Contributor

Choose a reason for hiding this comment

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

given that we never used the intermediate states, I'm fine changing the default behavior

@github-actions
Copy link

github-actions bot commented Jun 8, 2023

Coverage report

The coverage rate went from 85.68% to 84.92% ⬇️

80% of new lines are covered.

Diff Coverage details (click to unfold)

src/renate/defaults.py

100% of new lines are covered (99.03% of the complete file).

src/renate/benchmark/experimentation.py

75% of new lines are covered (93.45% of the complete file).
Missing lines: 388

@wistuba wistuba self-requested a review June 14, 2023 14:53
@wistuba wistuba merged commit 8023952 into dev Jun 16, 2023
@wistuba wistuba deleted the pt_delete_bkups branch June 16, 2023 06:29
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.

2 participants