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

Combine several jobs into one to save computation resources #131

Merged
merged 6 commits into from
Jan 19, 2024

Conversation

dachengx
Copy link
Collaborator

Develop a new ticket generator: combined_tickets_generator which combines combine_n_job jobs into one by " && ".

Because the initialization of the container and environment will take some time, which is a constant offset proportional to the number of jobs. So this PR will reduce those time.

@dachengx dachengx marked this pull request as ready for review January 13, 2024 09:36
Copy link

Pull Request Test Coverage Report for Build 7511654057

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.2%) to 90.599%

Totals Coverage Status
Change from base Build 7511624308: -0.2%
Covered Lines: 1407
Relevant Lines: 1553

💛 - Coveralls

Copy link

Pull Request Test Coverage Report for Build 7511725544

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.3%) to 90.599%

Totals Coverage Status
Change from base Build 7511724235: -0.3%
Covered Lines: 1407
Relevant Lines: 1553

💛 - Coveralls

Copy link

github-actions bot commented Jan 14, 2024

Pull Request Test Coverage Report for Build 7583552712

  • 10 of 15 (66.67%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.2%) to 90.677%

Changes Missing Coverage Covered Lines Changed/Added Lines %
alea/submitter.py 10 15 66.67%
Totals Coverage Status
Change from base Build 7556510982: -0.2%
Covered Lines: 1420
Relevant Lines: 1566

💛 - Coveralls

@hammannr
Copy link
Collaborator

@dachengx I quickly went over the code -- just for my understanding: Isn't this the same as just reducing the n_batch parameter?

@dachengx
Copy link
Collaborator Author

dachengx commented Jan 17, 2024

@hammannr I see your concern. Sorry, I was not clear enough in the description. Each job will have a unique nominal_value or generate_value for a parameter. Sometimes the number of combinations of different nominal_values and generate_values is very large. If I want to do a gridscan of two parameters' nominal_values, and each has 100 different choices, I will need to submit 10000 jobs. This PR mainly solves such a problem. If I set combine_n_jobs to 100, then I will only need to submit 100 jobs.

hammannr
hammannr previously approved these changes Jan 19, 2024
Copy link
Collaborator

@hammannr hammannr left a comment

Choose a reason for hiding this comment

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

Looks good! Two minor comments then you can go ahead and merge I'd say 😊

@@ -387,6 +388,31 @@ def already_done(self, i_args: dict) -> bool:
is_done = False
return is_done

def combined_tickets_generator(self):
"""Get the combined submission script for the current configuration. ``self.vcombine_n_job``
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
"""Get the combined submission script for the current configuration. ``self.vcombine_n_job``
"""Get the combined submission script for the current configuration. ``self.combine_n_job``

Copy link
Collaborator

Choose a reason for hiding this comment

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

could you maybe add a small example to make clear how/why one should use the combine_n_jobs? 😊

@dachengx dachengx merged commit 02aa46d into main Jan 19, 2024
7 checks passed
@dachengx dachengx deleted the combine_n_job branch January 19, 2024 12:14
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