ensure that cpp data structure for is_LC is the same in fortran and cpp#880
ensure that cpp data structure for is_LC is the same in fortran and cpp#880
Conversation
Hi Olivier, thanks a lot. Yes we have a CI, but I also have bypasses for 'known issues' to make sure that things are green otherwise (bit silly maybe... only needed till we fix everything). In your case now I think the bypasses are active, you need to comment out these three lines an din particular the middle one for 856 (Rephrasing: all tests are succeeding now, but it may be due to bypasses... though from the actual log I have the impression the test was actually succeesful. Comment out all three lines, you will have 6 tests failing if it worked, 9 failing if it did not work) |
|
Thanks, Let's hope the test will crash now (but only 6 times) :-) |
|
@valassi This should be ready for review. It might also be better to encapsulate the new line of code that I put in this plugin in the mg5amcnlo code (in a new function) to avoid repetition if you agree with that, then I can do it right away. Thanks, Olivier |
Thanks Olivier very good! I will test this (will take me a bit of time). Concerning the MG5AMC change Your take on this, I have no strong feeling. I think that the question is:
In the first case, you should probably NOT remove the line as otherwise export_cpp without cudacpp would not work. But I am not going to test that ;-) In the second case, which is probably the truth, then I guess it is indifferent. Eventually we might want to remove export_cpp and merge it into cudacpp, or viceversa (or in any case a good cleanup would be useful). But here it's really up to you to suggest/choose a direction... let me know! PS But ok, maybe all in all I have a slight prefernce for not removing the line. But again, your take... If we leave it in, the code will not crash in python, but will produce wrong results in generated code. Maybe what you were suggesting was to have a single implementation in export_cpp, ie move your current fix from cudacpp to export_cpp? That might make sense. BUt again your take... |
|
To answer your question, I need to distinct multiple part of export_cpp
So concerning the code style, I take your comment as a go ahead to do a bit of code refactorization, |
…er's madgraph5#880 for bug madgraph5#856) into color2
…that Olivier's PR madgraph5#880 fixes madgraph5#856 ./tmad/teeMadX.sh -ggttgg -makeclean +10x
…for issue madgraph5#856 - now including fixes in icolamp in coloramps.h files
…adgraph5#856 from known issues in the CI, after Olivier's fix in madgraph5#880
d580c9a to
85be439
Compare
|
I have updated and closed #856 (I tried to make it clear that it was closed here in #880, not in #877 or mg5amcnlo/mg5amcnlo#116) |
Ha ha dont worry :-) I am retesting my stuff (with plentu of rebase and force push and other uglier stuff!). Is this the final thing on your side? Can I merge this if my tests are ok? (Then I will later ask you to review #881) |
Maybe remove the draft status and mark it ready for review when ready please, then I will just merge (I confirm that it looks ok) |
…that Olivier's PR madgraph5#880 fixes madgraph5#856 ./tmad/teeMadX.sh -ggttgg -makeclean +10x
…for issue madgraph5#856 - now including fixes in icolamp in coloramps.h files
…adgraph5#856 from known issues in the CI, after Olivier's fix in madgraph5#880
|
Yes, it can be merged. Thanks a lot, Olivier |
|
Very good, thanks to you. Merging. |
… and madgraph5#881 fixes for madgraph5#856) into fpe
Adding a PR (in WIP) to see if the following branch does create issue.
@valassi do we have a CI test for the missmatch of color?
Because, the point of this PR is to be able to fix such missmatch (I did not tested that part yet).
In particular, I want to test if I did break things for SA side (It would be weird...)
The idea of such patch is to ensure that the datastructure use to write the color (is_LC information) is the same on the fortran and cpp side.
In the previous code, the function was called twice with two different type of data-structure. I do not understand why this was needed, so I fully removed the first call (which explains why I want to run the CI to check for side effect). (and change the datastructure of the second call)