Skip to content

change to pmap step in computeNextIteration#47

Merged
floswald merged 21 commits intofloswald:masterfrom
gregobad:better_parallel
May 5, 2025
Merged

change to pmap step in computeNextIteration#47
floswald merged 21 commits intofloswald:masterfrom
gregobad:better_parallel

Conversation

@gregobad
Copy link
Contributor

@gregobad gregobad commented Dec 6, 2019

Hi -

I modified the pmap step in computeNextIteration! to only parallellize the objective function evaluation step (not the accept / reject or proposal steps). This avoids having to pass the entire chain to the workers. I was running into problems where the evaluation slowed down over time and eventually failed with a bus error because the chain objects get very large. I think this should solve that problem.

Greg

…rob object plus proposed parameter vector (rather than entire chain object)
@floswald
Copy link
Owner

floswald commented Dec 9, 2019

sounds like a good idea! thanks!

@gregobad
Copy link
Contributor Author

This actually doesn't go far enough. pmap still gets passed a reference to the chain object, which is growing over time. Will update shortly to fix that.

@floswald
Copy link
Owner

floswald commented Jan 2, 2020

hey how are we doing with this? good to merge?

@gregobad
Copy link
Contributor Author

gregobad commented Jan 2, 2020

This change by itself doesn't give much of a memory improvement. To get any improvement it's necessary to use CachingPool. I couldn't figure out a way to do this cleanly. My current version of the code defines a global pool wp which gets passed to pmap in computeNextIteration!:

evs = pmap(x -> evaluateObjective(algo.m, x), wp, pps)

But this solution doesn't make a lot of sense for library use.

@gregobad
Copy link
Contributor Author

I've added an option to AlgoBGP.opts, "worker_pool", that allows the user to specify a worker pool to use for distributed computation. This gets passed to the pmap step in computeNextIteration!. This branch also has a few other additions, like the possibility of passing additional arguments to the objective function as keyword options.

@floswald
Copy link
Owner

floswald commented May 5, 2025

hey @gregobad , thanks for this. just checking, did you run the tests locally?

@floswald floswald merged commit eeb7cc7 into floswald:master May 5, 2025
2 checks passed
@floswald
Copy link
Owner

floswald commented May 5, 2025

🎉

@gregobad
Copy link
Contributor Author

gregobad commented May 5, 2025

I didn't run the tests locally but looks like they worked. I thought I was just pushing to my fork, didn't intend to create a pull request on the main branch. But perhaps the vector setMoments! is generally useful.

@floswald
Copy link
Owner

floswald commented May 5, 2025

sure we'll keep the vector version around, why not. took some tweaking to make the test run on x86.

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