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

Increase time resources #27

Merged
merged 9 commits into from
Jan 17, 2025
Merged

Increase time resources #27

merged 9 commits into from
Jan 17, 2025

Conversation

lazappi
Copy link
Contributor

@lazappi lazappi commented Jan 14, 2025

Describe your changes

Increase allocated time for methods failing for 143 exit codes

Checklist before requesting a review

  • I have performed a self-review of my code

  • Check the correct box. Does this PR contain:

    • Breaking changes
    • New functionality
    • Major changes
    • Minor changes
    • Bug fixes
  • Proposed changes are described in the CHANGELOG.md

  • CI Tests succeed and look good!

@lazappi lazappi requested a review from rcannood January 14, 2025 09:09
Copy link
Member

@rcannood rcannood left a comment

Choose a reason for hiding this comment

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

Do you think these methods might finish if they get twice the amount of time ? Otherwise all it'll do is make the benchmark take longer to finish without any extra results 😅

@lazappi
Copy link
Contributor Author

lazappi commented Jan 15, 2025

Hard to say but 4 hours isn't a lot of time for biggish datasets. I'm pretty sure at lease scanorama should work. I think it will help for at least some of them.

@rcannood
Copy link
Member

Alright, makes sense. If we bump the time limits for some components, I think we should bump it for all methods to ensure that the different methods get a level playing field when new datasets get added

@lazappi
Copy link
Contributor Author

lazappi commented Jan 16, 2025

So I should set hightime for all the methods?

…resources

* origin/main:
  Bump scIB to v1.1.7 (#30)
  Pin scPRINT version (#25)
  Increase UCE memory tag (#24)
  Add exit codes helper file (#26)
  Include all features in solution (#28)
  Update common submodule (#29)
@rcannood
Copy link
Member

So I should set hightime for all the methods?

I'd sure the same time label across all methods -- it's a bit unfair if we don't

@rcannood rcannood merged commit 0d4d0d0 into main Jan 17, 2025
1 check passed
@rcannood rcannood deleted the bugfix/no-ref/adjust-resources branch January 17, 2025 11:25
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