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

MAINT: Use a large resource class to speed up build #741

Merged
merged 4 commits into from
Jun 15, 2023

Conversation

larsoner
Copy link
Member

Interested in trying a large resource class since ERP_CORE_* (e.g. ERN) appears to be CPU-bound with 2 CPUs. First I'm pushing an empty commit just to make sure CIs are green and replicate the ~19 minute timing. Then we'll try a large resource class to see if it helps!

@hoechenberger
Copy link
Member

hoechenberger commented Jun 15, 2023

I'd like to ensure we keep the same amount of memory though. These resource limits have helped me discover inefficient memory utilization in the past

@larsoner
Copy link
Member Author

That would be a bit harder. We could run with ulimit maybe. Let's see if the speed gains are there first. Then we can decide if the tradeoff of increased memory limit is worth it.

Actually thinking about it the n_jobs to dask have a fixed limit of 2gb in their config so it should be safe enough in those jobs at least -- and those are the ERP CORE ones that will probably benefit most!

@larsoner
Copy link
Member Author

Okay that did take it from 35 minutes down to 30, which is nice. Looks like all the ERP_CORE got faster, and it should be memory-safe to increase the resource class here since (for parallel at least) we use a memory limit on the dask workers:

no-op commit resource-class: large
Screenshot from 2023-06-15 13-33-08 Screenshot from 2023-06-15 13-33-18

There wasn't much effect for the others (e.g., ds000248) though, so I'll revert those then merge.

@larsoner
Copy link
Member Author

... actually 1810 went from 12 to 8 min which seems worth it, too. So the effect is that just one of the tests (1810) is run no longer as memory limited, which seems worth it

@larsoner larsoner changed the title TST: Try different resource classes MAINT: Use a large resource class to speed up build Jun 15, 2023
@larsoner larsoner enabled auto-merge (squash) June 15, 2023 17:40
@larsoner larsoner merged commit 416ac0f into mne-tools:main Jun 15, 2023
@larsoner larsoner deleted the circle branch June 15, 2023 18:38
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