-
Notifications
You must be signed in to change notification settings - Fork 25
Cell search #156
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
Cell search #156
Conversation
Added in the BEAVRS model with the D-bank partially inserted. Also fixed a flaw in the geometry where the outermost cell was defined such that there could be a rare particle lost between it and the geometry boundary.
Can enable overlap checking in cell universes. Required adding an overlap material. In adding this, also fixed a few typos, updated docs, and changed transport operators to use select cases rather than several ifs.
|
I'm sorry to make this bigger. I have added overlaps. However, these were closely associated with the cellUniverse. Additional modifications to other files are very small, I promise. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I confirm that anything geometry related just breaks me......
Other than minor aesthetic comments, a couple of more high level questions:
-
what is the point of linked lists? I haven't read Harper's paper, but to me it seems like the same function could have been done with our dynArr, for example.
-
I understand why the semaphore thing is a bit uncomfortable.. Ideally, the reordering would happen occasionally at the end of cycle so that only one thread looks at it - but the interface change is annoying. I'm not sure I have a better idea there.
-
I am also not fully convinced by the criterion for reordering. You check the frequency of the local cell being visited, rather than total number of cell visits.
-
Could you maybe save an old cell seach list and a new one? Now, during cell reordering, the other threads go back to an ordered list. If you could provisionally save the old list, then the other threads could use that instead during reordering. This however might be a waste of memory and buy very little if the cell reordering is fast.
Also, I haven't comment on the BEAVRS inputs but I leave it to you to fix the conflicts!
Co-authored-by: valeriaRaffuzzi <108435337+valeriaRaffuzzi@users.noreply.github.com>
Co-authored-by: valeriaRaffuzzi <108435337+valeriaRaffuzzi@users.noreply.github.com>
Co-authored-by: valeriaRaffuzzi <108435337+valeriaRaffuzzi@users.noreply.github.com>
Co-authored-by: valeriaRaffuzzi <108435337+valeriaRaffuzzi@users.noreply.github.com>
Co-authored-by: valeriaRaffuzzi <108435337+valeriaRaffuzzi@users.noreply.github.com>
Co-authored-by: valeriaRaffuzzi <108435337+valeriaRaffuzzi@users.noreply.github.com>
Co-authored-by: valeriaRaffuzzi <108435337+valeriaRaffuzzi@users.noreply.github.com>
Co-authored-by: valeriaRaffuzzi <108435337+valeriaRaffuzzi@users.noreply.github.com>
Co-authored-by: valeriaRaffuzzi <108435337+valeriaRaffuzzi@users.noreply.github.com>
…into cellSearch
Co-authored-by: valeriaRaffuzzi <108435337+valeriaRaffuzzi@users.noreply.github.com>
Co-authored-by: valeriaRaffuzzi <108435337+valeriaRaffuzzi@users.noreply.github.com>
|
Sorry, forgot to respond to comments above. Agreed on not really knowing what to do with the geometry update. I suggested in one of the comments that we might want to make a generic 'geometryUpdate' subroutine for time-dependent things, but even that isn't perfect - for time-dependent we probably want to update geometry (or have a time-dependent geometry) that would look different during a cycle. I am not convinced about the reordering criterion either. This was plucked from the air. There may be better ways of doing it, but for now this offered a meaningful improvement. This could be optimised further later. That is a good idea on what to do during reordering. However, I think that may also be worth investigating later since for now everything seems to perform well. |
valeriaRaffuzzi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is ok for now then! Thanks for the implementation :)
I've implemented neighbour lists and cell search sorting (as in Serpent) to speed up particle transport in large cell universes.
The cell search reordering, on an ATR model, gave a >2x speed-up for 200k particles per cycle when doing a pure delta tracking run. The neighbour list gives a ~60x speed-up for the same system when doing a pure surface tracking run.
The neighbour list required implementing a linked list (following the recommendations from Harper et al). This is likely not implemented optimally.
Additionally, cell reordering is challenging to do in a thread-safe manner. This is because, to prevent breaking interfaces, it needs to be done during particle transport. I have done it by implementing something like a semaphore: a flag that says when reordering is happening, use naive cell ordering. This feels like it could be somewhat risky... Would be glad to hear any further thoughts on this.