Skip to content

Multithreaded sky map operations#219

Merged
arahlin merged 12 commits intomasterfrom
multithread-remove-weights
Mar 19, 2026
Merged

Multithreaded sky map operations#219
arahlin merged 12 commits intomasterfrom
multithread-remove-weights

Conversation

@LiYunyang
Copy link
Collaborator

@LiYunyang LiYunyang commented Feb 2, 2026

Use OpenMP for map operations that are thread-safe (i.e. those that do not require resizing sparse backend storage). This includes any math operations on dense maps, populating sky masks, or applying map weights.

@LiYunyang LiYunyang requested a review from arahlin February 2, 2026 01:38
@LiYunyang LiYunyang self-assigned this Feb 2, 2026
@arahlin
Copy link
Member

arahlin commented Feb 4, 2026

Are there any other loops of this sort in the maps package that we should parallelize? Might as well include any others in this PR too.

@LiYunyang
Copy link
Collaborator Author

The other case is the zero_nans branch of RemoveWeightsT. I just changed it too.

There are a couple of instances in maputils.cxx that have the same structure, such as getXXMask. I suppose they are not called as frequently (as opposed to coadd) to make this a bottleneck.

@arahlin
Copy link
Member

arahlin commented Mar 18, 2026

There are a couple of instances in maputils.cxx that have the same structure, such as getXXMask. I suppose they are not called as frequently (as opposed to coadd) to make this a bottleneck.

I would probably just parallelize all those loops anyway for consistency.

(Apologies for delayed review!)

@LiYunyang LiYunyang force-pushed the multithread-remove-weights branch from e44615d to 2037709 Compare March 19, 2026 02:09
@LiYunyang
Copy link
Collaborator Author

I searched for all cases of for (size_t and changed it for GetRaDecMap, GetRaDecMask, and GetGalacticPlaneMask -- I don't think the rest are thread-safe.

While doing this, I was warned that a race condition can occur if the map is not in a dense format (this is why ReprojMap fails ). Sasha, do you think this is a concern for RemoveWeights and ApplyWeights?

@arahlin
Copy link
Member

arahlin commented Mar 19, 2026

I searched for all cases of for (size_t and changed it for GetRaDecMap, GetRaDecMask, and GetGalacticPlaneMask -- I don't think the rest are thread-safe.

While doing this, I was warned that a race condition can occur if the map is not in a dense format (this is why ReprojMap fails ). Sasha, do you think this is a concern for RemoveWeights and ApplyWeights?

Ah, that's a really good point, and probably why this wasn't parallelized in the first place. There is almost certainly a way to make the operation thread-safe for non-dense formats, but would take quite a bit of additional care. I haven't thought about this in any detail, but perhaps there are certain operations that can be marked as critical or atomic.

As it is, I think using this for ApplyWeights is fine, because that operation doesn't create any new non-zero pixels (that's the part that is not thread-safe, because it modifies the storage structure). For RemoveWeights, the zero_nans branches should probably not be parallelized until we can figure out how to handle non-dense storage.

@arahlin arahlin changed the title Multithread remove weights Multithreaded sky map operations Mar 19, 2026
@arahlin arahlin merged commit 84a3072 into master Mar 19, 2026
2 checks passed
@arahlin arahlin deleted the multithread-remove-weights branch March 19, 2026 18:57
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