[BUG] replace combine with itertools product#253
Conversation
|
@fkiraly we've been getting lots of failures on setup in macos and windows like this: https://github.com/dswah/pyGAM/actions/runs/19465120143/job/55698192192 they all complain about codecov. |
|
Honestly, I have no idea what is going on with codecov, or why moving it to the end would change anything - sorry. Do the search engines or AIs say anything useful about these errors? |
|
|
||
| # loop through candidate model params | ||
| for param_grid in pbar(param_grid_list): | ||
| for grid in pbar(product(*grids), max_value=grid_size): |
There was a problem hiding this comment.
Do you think it might be beneficial to parallelize this for loop? I can create an issue/PR if that might be interesting.
There was a problem hiding this comment.
Hey @ankurankan I think that would be great!
Do you think it will run faster?
I have never been very good art parallelizing numerical python routines. My parallel routines always seem to run just as fast/slow as my serial ones...
There was a problem hiding this comment.
I can give it a quick try and share some benchmarks. Let's see if that improves anything. Have created an issue: #406
for large search-spaces, gridsearch can easily exhaust all available memory before fitting a single model.
This is because the method tries to pre-determine the entire search space in memory.
we could instead use a generator like
itertools.productthis fixes #242
TO-DO
pygam.utils.combine