phoebe-project / phoebe2

PHOEBE - Eclipsing Binary Star Modeling Software
http://phoebe-project.org
GNU General Public License v3.0
80 stars 30 forks source link

Issue with broadcasting of variables in MPI even when PHOEBE_ENABLE_MPI is set to FALSE #845

Open abhimat opened 7 months ago

abhimat commented 7 months ago

I'm encountering an issue with PHOEBE broadcasting the following variable during import when I run it within MPI, but I want to not use MPI and parallelization within PHOEBE. This is lines 164–168 in phoebe/__init__.py.

    def off(self):
        if self.within_mpirun and self.myrank == 0:
            self.comm.bcast({'worker_command': 'release'}, root=0)

        self._enabled = False

My use case is that I am using UltraNest for fitting, and I am using MPI for parallelizing the sampling portion. Therefore, I want PHOEBE to not use its own parallelization, and instead only have the UltraNest sampling be parallelized.

I set the environment variable PHOEBE_ENABLE_MPI to FALSE, but I still get an issue with the broadcast of {'worker_command': 'release'} in the lines quoted above. In my case, it interferes with the use of MPI within UltraNest, but I believe this would interfere with any other wrapper around PHOEBE that is attempting to use MPI outside of PHOEBE.

Commenting out the broadcast lines above is now allowing my binary fitting code to work fine with UltraNest. But this seems like a possible bug with PHOEBE to be broadcasting even if PHOEBE itself is not supposed to be using MPI?

Would it be okay to remove that bcast() line, or is it necessary for something else that PHOEBE needs? Wanted to bring it to your attention in case removing that bcast() line might be unintentionally breaking something else that I'm not yet understanding! Thanks!

kecnry commented 7 months ago

This logic is intended to release all other (non-zero ranked) processes from the while loop and is necessary for cases where phoebe is started within an MPI environment (and MPI is not already disabled). Since that while loop is avoided by setting the environment variable, it should be safe to skip the broadcast. I think the actual solution will be to modify that if statement to:

if self.within_mpirun and self.enabled and self.myrank == 0:

Can you check if that works for your use-case here (and that ultranest is getting access to all the processors as expected)? If so, I can open a quick PR to get this in the next bugfix release. Thanks for bringing this to our attention!

If you have success using ultranest with phoebe, we'd also love to hear about that! And if you or anyone would have interest in contributing an official fitting backend within phoebe, I'd be happy to help you get started. See https://github.com/phoebe-project/phoebe2/issues/414 and https://github.com/phoebe-project/phoebe2/discussions/407#discussioncomment-359833 for some existing discussion already.

abhimat commented 7 months ago

Thank you, that solution does indeed work correctly for me when I use phoebe with ultranest sampling through MPI!

I have been successfully using ultranest and phoebe, but this is with my own wrapper package around phoebe that also implements some of our research project’s synthetic photometry needs!

I don't yet have much experience with using the sampling backends within phoebe, but would be happy to try helping implement ultranest within phoebe if I can!