Support for CellInstanceFilter#88
Conversation
| # Iterate over tally bins, setting any pixels that have matching (cell | ||
| # ID, instance) each time | ||
| for v, (cell_id, instance) in zip(data, bins): | ||
| cell_id_mask = (self.cell_ids == cell_id) |
There was a problem hiding this comment.
I'm not sure how much of a performance hit this might be, but having this in the loop is going to cause a repeated calculation of this mask for DistribCellFilter's. The same is true for CellInstanceFilter's that contain instances of the same cell.
I suppose that since we're typically working with around 100k pixels in the image data this shouldn't be too bad. Mainly noting it here in case we run into a performance issue with these filter types in the future.
There was a problem hiding this comment.
Yup, I'm well aware that this is performance-shitty code. But, so far on the models I've played with it doesn't seem to be a big deal. It would be easy enough to optimize if we find later on that it is causing significant lags on larger models. I can go ahead and add a comment in the code about this since I'm sure we'll probably forget.
pshriwise
left a comment
pshriwise
left a comment
There was a problem hiding this comment.
I think this looks good @paulromano! The formatting cleanup is appreciated and I like the way you were able to use the distribcell function.
2 participants
This PR is a quick follow-up to #87 that extends support to
openmc.CellInstanceFilteras well. I've managed to do it in a way that requires very little extra code. Technically it makes the logic for distribcell plotting slightly less efficient, but I'm not sure it really matters.Closes #86