wrong type argument to unary minus (FFV functions should not use "-COUPs" as argument) #628
Comments
|
Took a quick look at other processes where I thought this might occur, and found a minimal example in the electroweak sector of the SM: Same error appears when trying to build from the P1_ directory. |
Thanks Zenny! Which process is this? As mentioned above, I saw this in gq to ttllq SM (and in a SUSY process) This is clearly high priority - but complex potentially. Need to analyse the Fortran/Python code and discuss with @oliviermattelaer |
Ah, forgot to mention that! It's Just to make sure, I also checked that the charge-conjugated process |
|
Note I stumbled also over this in the "CMS process" from Sapta (Drell Yan). I worked around it with turning the signs inside the array. |
|
Yes even in pure fortran, the compiler complains about unary operation, but this is just a warning. So should I write this then? Cheers, Olivier |
|
Hi Olivier, thanks for looking at this. No it is more complex than that (and not very obvious from the code manybe, sorry). The point is that "COUPs[1]" is a pointer, not a value. The value would be in "COUPs[1][i]", where i is an event id if I remember correctly. It must be a pointer because we have the running G and so all couplings are event-dependent, they are not constants. What would be needed is to make sure that the minus sign goes INSIDE the FFV implementation, rather than in the function calling the FFV. But I am not sure this is possible. Various alternatives I considered (just brainstorming) could be
For a given coupling in a given FFV function, would all callers of that function always use one of COUP or -COUP, or are there multiple callers and some may have COUP and some -COUP? Thanks |
|
There may be a third option, the problem why negating the coupling constant doesn't work right now is because its an array which represents a complex number. IIRC each function parameter represents one complex number (an array of 2). The proposal would be to create the complex number outside the function in a global and then let the complex unary minus operator do the job if needed when the global is passed as a function parameter. |
Hi, thanks for the feedback. This is useful but not correct. The problem, in general, is not that these arrays represent complex numbers. The problem is that all FFV functions now take ONLY arguments which are arrays. This is by design, to make sure that all FFVs have uniform signatures and can be handled using similar decoding mechanisms. Now, in one example I checked (the gq_ttllq), it turns out that the "-COUPs[3]" refers to an alphas-independent coupling. So you are kind of right that in this case it is a single complex number, because the same value is used for all events. Note however that the design of this thing is presently meant to use a pointer even if we had a single real number, not even a complex. So, again, the problem is that FFVs take pointers by design, not values. And you cannot do a unary minus of a pointer. Let me rephrase: in designing this I was assuming (assumption which was correct for all of our initial processes) that the FFVs would be called with a coupling pointer, and that then it would be the FFV itself that would do "minus the coupling" if needed. This is now shown to be a wrong assumption. Now there were three options, and the first one is wrong, but there are still two possibilities
@oliviermattelaer can you try to clarify please? Concerning a "more general" solution, I saw that gq_ttllq actually has some FFV calls with "COUPs[3]" an donly some other FFV calls (always the same ones, eg FFV2_5_0) with "-COUPs[3]". So one option I was considering, namely having only "-coupling" in the COUPs[3] pointer, would not work, as we need both positive and negative couplings. Also about a more genearl solution, I checked that presently the template argiuments for FFVs include only one CI_ACCESS accessor. The same is used for instance for COUPs[3] and COUPs[5] in Again, the easiest would be if we were sure that FFV_2_5_0 calls always come with "-COUPs[3]" or similar (the first coupling) with a minus sign, then the minus sign could be moved inside the implementation. But this is very unlikely. I think we most likely need an extra argument. I can "easily" do that in the cudacpp code generation, but its tedious and error prone, I would need around one week I think. Essentially: should become either this, with one extra template argument, for instance differemt accessors if the coupling must get an extra minus sign or not, or something like Any other thoughts? Thanks |
|
Hi,
The presence of the "-" is technically an optimization trick.
In the past, we never had such minus sign but had two coupling
for example:
GC_11 = aS
GC_12 = -aS
and in that way FFV was called with either GC_11 or GC_12.
In modern computation GC_12 does not exist anymore and we pass
"-GC_11" instead.
This can happen both for coupling dependent and/or independent of alpha_s.
(and in theory one can think that at some point it is not only for -1 but also for other simple complex number)
Again, the easiest would be if we were sure that FFV_2_5_0 calls always come with "-COUPs[3]" or similar (the first coupling) with a minus sign, then the minus sign could be moved inside the implementation. But this is very unlikely.
Yeah this unlikely to work for any model/....
I think we most likely need an extra argument. I can "easily" do that in the cudacpp code generation, but its tedious and error prone, I would need around one week I think.
That's one solution, the other one is to "undo" the optimization and to restore those "GC_12" within the model...
(This should be simple but will have a cost in term of memory)
Cheers,
Olivier
On 22 May 2023, at 15:27, Andrea Valassi ***@***.***> wrote:
There may be a third option, the problem why negating the coupling constant doesn't work right now is because its an array which represents a complex number.
Hi, thanks for the feedback. This is useful but not correct. The problem, in general, is not that these arrays represent complex numbers. The problem is that all FFV functions now take ONLY arguments which are arrays. This is by design, to make sure that all FFVs have uniform signatures and can be handled using similar decoding mechanisms.
Now, in one example I checked (the gq_ttllq), it turns out that the "-COUPs[3]" refers to an alphas-independent coupling. So you are kind of right that in this case it is a single complex number, because the same value is used for all events. Note however that the design of this thing is presently meant to use a pointer even if we had a single real number, not even a complex. So, again, the problem is that FFVs take pointers by design, not values. And you cannot do a unary minus of a pointer.
Let me rephrase: in designing this I was assuming (assumption which was correct for all of our initial processes) that the FFVs would be called with a coupling pointer, and that then it would be the FFV itself that would do "minus the coupling" if needed. This is now shown to be a wrong assumption.
Now there were three options, and the first one is wrong, but there are still two possibilities
1. I was assuming FFVs would be called with coupling pointer for all couplings, dependent and independent. This is wrong.
2. Maybe, maybe, FFVs are sometimes called with "-coupling" only for alphas-independent couplings? This is what I see in gq_ttllq (I have not even checked susy). If this was the case, then maybe the best option is to treat independent couplings differently from dependent couplings, and use values for independent couplings, pointers for dependent couplings. This would be quite ugly, but would work. But is this assumption correct?
3. If, conversely, also alphas-dependent couplings are sometimes called as "-COUPs" when invoking FFVs, then we need a more general solution. Investing time for some quick patch along option 2 would just be a waste of time.
@oliviermattelaer<https://github.com/oliviermattelaer> can you try to clarify please?
Concerning a "more general" solution, I saw that gq_ttllq actually has some FFV calls with "COUPs[3]" an donly some other FFV calls (always the same ones, eg FFV2_5_0) with "-COUPs[3]". So one option I was considering, namely having only "-coupling" in the COUPs[3] pointer, would not work, as we need both positive and negative couplings.
Also about a more genearl solution, I checked that presently the template argiuments for FFVs include only one CI_ACCESS accessor. The same is used for instance for COUPs[3] and COUPs[5] in FFV2_5_0<W_ACCESS, A_ACCESS, CI_ACCESS>( w_fp[3], w_fp[14], w_fp[11], -COUPs[3], COUPs[5], &_fp[0] );. This is a problem because this call mixes one negative "-COUPs[3] and one positive 'COUPs[5]'. So one cannot use the template argument CI_ACCESS to give one - and one +, as it is the same CI_ACCESS for both. It really seems that we need one extra argument to the FFV calls, either in the template list or in the function arguments.
Again, the easiest would be if we were sure that FFV_2_5_0 calls always come with "-COUPs[3]" or similar (the first coupling) with a minus sign, then the minus sign could be moved inside the implementation. But this is very unlikely.
I think we most likely need an extra argument. I can "easily" do that in the cudacpp code generation, but its tedious and error prone, I would need around one week I think. Essentially:
FFV2_5_0<W_ACCESS, A_ACCESS, CI_ACCESS>( w_fp[3], w_fp[14], w_fp[11], -COUPs[3], COUPs[5], &_fp[0] );
should become either this, with one extra template argument, for instance differemt accessors if the coupling must get an extra minus sign or not,
FFV2_5_0<W_ACCESS, A_ACCESS, CI_ACCESSm, CI_ACCESSp>( w_fp[3], w_fp[14], w_fp[11], COUPs[3], COUPs[5], &_fp[0] );
or something like
FFV2_5_0<W_ACCESS, A_ACCESS, CI_ACCESS>( w_fp[3], w_fp[14], w_fp[11], -1, COUPs[3], +1, COUPs[5], &_fp[0] );
Any other thoughts? Thanks
Andrea
—
Reply to this email directly, view it on GitHub<#628 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AH6535R2WWB7BT6HWLHII2DXHNSUHANCNFSM6AAAAAAWRW46VY>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
|
Hi Olivier, thanks a lot. This clarifies.
Ok then we definitely cannot assume that this only happens for independent couplings. So my option 2 would not work, we need some moregeneral solution as in my option 3.
Yes I agree. I think this is probably the best solution. It is essentially equivalent to one of the ideas I threw on the table (keep both +coupling and -coupling in two separate arrays), but the advantage is that you can do that with the physics in mind and maybe this still has some optimizations? I mean, if you know that in a process you need only GC11=+aS and you do not need GC12=-aS, I guess you would only use GC11 and would not introduce GC12? Is there a quick way I can 'disable' the optimization at code generation time? Thanks! |
|
Here is a dirty patch (that will impact everything... so we clearly want something cleaner but as a quick work around it should be ok)
dans models/import_ufo.py
at line 2234:
if value in dict_value_coupling or -1*value in dict_value_coupling:
if value in dict_value_coupling:
iden_key.add(value)
dict_value_coupling[value].append((name,1))
else:
iden_key.add(-1*value)
dict_value_coupling[-1*value].append((name,-1))
else:
dict_value_coupling[value] = [(name,1)]
If you remove all the "-1*value" this should have that impact ( I guess).
Olivier
On 22 May 2023, at 16:34, Andrea Valassi ***@***.***> wrote:
Hi Olivier, thanks a lot. This clarifies.
The presence of the "-" is technically an optimization trick. In the past, we never had such minus sign but had two coupling for example: GC_11 = aS GC_12 = -aS and in that way FFV was called with either GC_11 or GC_12. In modern computation GC_12 does not exist anymore and we pass "-GC_11" instead. This can happen both for coupling dependent and/or independent of alpha_s.
Ok then we definitely cannot assume that this only happens for independent couplings. So my option 2 would not work, we need some moregeneral solution as in my option 3.
That's one solution, the other one is to "undo" the optimization and to restore those "GC_12" within the model... (This should be simple but will have a cost in term of memory)
Yes I agree. I think this is probably the best solution. It is essentially equivalent to one of the ideas I threw on the table (keep both +coupling and -coupling in two separate arrays), but the advantage is that you can do that with the physics in mind and maybe this still has some optimizations?
I mean, if you know that in a process you need only GC11=+aS and you do not need GC12=-aS, I guess you would only use GC11 and would not introduce GC12?
Is there a quick way I can 'disable' the optimization at code generation time?
Thanks!
Andrea
—
Reply to this email directly, view it on GitHub<#628 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AH6535Q5TQBYIHHBH34CQFTXHN2OZANCNFSM6AAAAAAWRW46VY>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
Thanks Olivier, I will give it a try. Just speculating, in order to make a "cleaner" patch can we pass somehow an option to the code in import_ufo? I see there is this for instance Are these options something you specify when you import a model? It would force users to add something to the import model, but it would already be better I think. Even better, it would be nice to do this only inside the cudacpp (and not in the fortran only part), but I imagine this is really difficult. I guess we import the model once and that's it, so for instance when we create both fortran and cudacpp MEs then the sam emodel must be used for both...? |
|
Hi @oliviermattelaer I am keeping this for later, but while working on something else I noticed this in ggtt logs for instance Is this 'fuse the coupling' the feature you were describing above, or is it something else? (By the way is this 'fusing' something that only applies to independent couplings or also to alphas-dependent couplings?) |
|
Yes it is that feature. |
|
Great, thanks a lot |
…las" and "cms" process examples added by Stefan but move them to the end of the list Note: the net effect of this MR is to add atlas and cms to the generateAndCompare.sh script, without fixing their builds - the cms process is affected by the "unary minus" issue (madgraph5#628) - the atlas process must be built with HRDCOD=1 because it has a modified SM with zero b mass (similar to madgraph5#616)
…las" and "cms" process examples added by Stefan but move them to the end of the list Note: the net effect of this MR is to add atlas and cms to the generateAndCompare.sh script, without fixing their builds - the cms process is affected by the "unary minus" issue (madgraph5#628) - the atlas process must be built with HRDCOD=1 because it has a modified SM with zero b mass (similar to madgraph5#616)
|
Discussion today:
|
…Olivier's patch for unary minus madgraph5#628 All FFV functions now have an extra argument Ccoeff in their signature. I checked with a quick test that tput and tmad tests look ok for gg_tt (logs not saved) ./tput/teeThroughputX.sh -ggtt -makeclean -makej ./tmad/teeMadX.sh -ggtt +10x There appears to be a ~10% improvement in performance in ggtt tests? To be tested on the other processes... NB: I also quickly tried to see if the unary minus isse itself has disappeared. This was failing with 'unary minus' issues before Olivier' patch, it now fails elsewhere after the patch. It seems that the 'unary minus' issue madgraph5#628 itself has been fixed. ./CODEGEN/generateAndCompare.sh susy_gg_tt cd susy_gg_tt.sa/SubProcesses/P1_Sigma_MSSM_SLHA2_gg_ttx/ make -j HRDCOD=1 |& grep unary
wrong type argument to unary minus (FFV functions should not use "-COUPs" as argument)
I have seen this issue in two places
For SUSY for instance
586195d
This is potentially tough to solve. I see two ways
Copying @zeniheisser as I understand this might be a showstopper for SUSY studies. Sopying also @oliviermattelaer
The text was updated successfully, but these errors were encountered: