-
Notifications
You must be signed in to change notification settings - Fork 32
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
(june24) check if anything needs to be changed in dsample.f in the use of vecsize_used or nb_warp_used #983
Comments
Hi @oliviermattelaer I am looking at this. These are currently the gg_tt.mad versions (after removing dead code and after indenting)
Old upstream/master
New current june24
|
Hi @oliviermattelaer I had a look at the two pieces of code above. My fortran is rusty (and it took me quite some time to understand the logic of the lagorithm), but all in all it looks correct, in both versions? Rephrasing: is there something that you think should be changed? To be specific:
Even more specifically, the magic is here and is the same in old and new code
Until ivec<vecsize_used, only the part above is done (one event at a time). When ivec becomes vecsize_used, the part below (where there are vector APIs, with calls for bunches of vecsize_used events) is executed, but at the same time ivec=0 is reset, so that the next DO will initially fill a bunch. Then in the vector part (the part 'below'), either vector APIs are used, or explicit loops over vecsize_used events. So all looks ok. One final comment for my own sanity, passcuts takes a vecsize_used parameter but is actually mainly a scalar event by event call. The parameter is passed because on the "first" call there is some update_param for a full bunch of events. This part of the code looks really dangerous to me. There is a mixture of local FIRSTTIME and a common CUTSDONE. What should be a simple "momenta in for one event, falg true/false out for one event" becomes something whose state depends on a common which is initialized and reset not clear where? (That said, I even wonder if I introduced some of this myself 2-3 years ago? I ceratinly added the vecsize_used as a parameter, because previously it was a vecsize_memmax from a parameter... but definitely the mix of scalar and vector behaviour with commons was already there). Maybe this could be reviewed als to make sure there are no bugs? (see #987) Voila. My summary: I am unable to see a problem here about a nb_warp_used missing somewhere. So I would merge june24. But please have a look yourself too Olivier. Thansk! |
Ok convincing argument. Well done So let me close this issue and let merge the associated PR |
No need to apologize Olivier, I very much prefer false alarms than missing real issues! Thanks so much to you, let's move on. I will start merging elsewhere... |
Just a reminder for myself from the chat with @oliviermattelaer this afternoon
=> Check if vecsize_used and nb_warp_used are needed in dsample.f
The text was updated successfully, but these errors were encountered: