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

fix(ringmapmaker): MPI calls could get out of sync #142

Merged
merged 9 commits into from
Jul 8, 2021
Merged

Conversation

jrs65
Copy link
Contributor

@jrs65 jrs65 commented Jun 16, 2021

This fixes a nasty bug where MPI calls could get out of sync when using
DeconvolveAnalyticalBeam in a case where different ranks have
differing numbers of frequencies. To rectify this it refactors the code
such that the beam_mmodes are generate for all frequencies at once which
will prevent the bug from resurfacing in newer implementations.

@jrs65 jrs65 requested a review from ssiegelx June 16, 2021 01:21
Copy link
Contributor

@ssiegelx ssiegelx left a comment

Choose a reason for hiding this comment

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

This looks great. I just have one comment.

draco/analysis/ringmapmaker.py Outdated Show resolved Hide resolved
draco/analysis/transform.py Show resolved Hide resolved
jrs65 added 9 commits July 7, 2021 23:14
This fixes a nasty bug where MPI calls could get out of sync when using
`DeconvolveAnalyticalBeam` in a case where different ranks have
differing numbers of frequencies. To rectify this it refactors the code
such that the beam_mmodes are generate for all frequencies at once which
will prevent the bug from resurfacing in newer implementations.
This allows using a grid beam with a different grid to the underlying
data. This can be applied as an external beam when deconvolving ringmaps
provided it is higher resolution than the data.
A bug meant that the high-m modes could potentially be left unset as the
array was not filled at creation, and the highest modes might not
actually be set.
@jrs65
Copy link
Contributor Author

jrs65 commented Jul 8, 2021

@ssiegelx I think this is done. I added a new oddra property to MContainer which tracks if it came from a odd/even number of RA points. This allows the containers to be exactly invertible without hackish checks.

@jrs65 jrs65 merged commit 56d1f1e into master Jul 8, 2021
@jrs65 jrs65 deleted the fix-ringmap-mpi branch July 8, 2021 06:27
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