Skip to content
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

clang-format (and related code changes) #388

Merged
merged 189 commits into from
Mar 3, 2022
Merged

Conversation

valassi
Copy link
Member

@valassi valassi commented Feb 24, 2022

Hi @roiser, this is my first draft proposal for clang-format (see #49).

Done:

  • completed .clang-format
  • formatted ggttgg code using that .clang-format (and additional manual changes including very few on/off blocks)

To do:

  • test that the code builds and runs (not yet done...)
  • backport to code generation
  • regenerate all processes
  • add to my generation scripts some checks that the clang-format is being respected
  • (optionally: for someone else? add a merge hook to check that the code is respected?)

Have a look if you have comments! Thanks Andrea

(/cvmfs/sft.cern.ch/lcg/releases/clang/13.0.0-7985e/x86_64-centos7/bin/clang-format -style=xxx -dump-config > .clang-format.xxx)
…sft.cern.ch/lcg/releases/clang/13.0.0-7985e/x86_64-centos7/share/clang/clang-format.el
…elAmps.h and read_slha.cc stay stable!

(however HelAmps.h would have looked differently with this .clang-format starting from upstream/master...)
Note that 'std::cout << "parameter " << "=" <<' gets split onto two lines...
I would need to change this as 'std::cout << "parameter =" <<'
This is a bug, see llvm/llvm-project/issues/44363
Revert "[clf] format Parameters_sm.cc"
This reverts commit 77ba20b.
…t off" comments

(it is difficult to clang-format macros...)
@valassi
Copy link
Member Author

valassi commented Mar 2, 2022

I have now done

  • fixed a functional bug (I had hardcoded a color=24)
  • regenerated all 5 main processes, did a first basic test of performance
  • analysed and fixed a performance regression in eemumu: I had added a few extra parentheses as "( TMP4 ) + ( - cI )" as a workaround to avoid expressions like "( TMP4 ) - cI" that clang-format (BUG?!) interprets as a C cast "(TMP4)-cI", and this and similar issues were adding a performance overhead; now fixed as "TMP4 - cI" in code generation, recovering performance

In progress

  • other non SM processes

TODO

  • complete non SM processes
  • regenerate all 5 processes with final formatting, and rerun all performance tests

@valassi
Copy link
Member Author

valassi commented Mar 2, 2022

About other processes, a few issues to fix:

diff bb_tt.auto/SubProcesses/P1_Sigma_sm_bbx_ttx/CPPProcess.cc.badfmt bb_tt.auto/SubProcesses/P1_Sigma_sm_bbx_ttx/CPPProcess.cc
171,172c171,172
<       jamp_sv[0] += +1. / 6. * amp_sv[0];
<       jamp_sv[1] -= 1. / 2. * amp_sv[0];
---
>       jamp_sv[0] += +1./6. * amp_sv[0];
>       jamp_sv[1] -= 1./2. * amp_sv[0];

diff pp_tt.auto/SubProcesses/P1_Sigma_sm_uux_ttx/CPPProcess.cc pp_tt.auto/SubProcesses/P1_Sigma_sm_uux_ttx/CPPProcess.cc.badfmt 
181,182c181,182
<       jamp_sv[0] += +1./6. * amp_sv[0];
<       jamp_sv[1] -= 1./2. * amp_sv[0];
---
>       jamp_sv[0] += +1. / 6. * amp_sv[0];
>       jamp_sv[1] -= 1. / 2. * amp_sv[0];
511c511
<     const int denominators = 36,36; // FIXME: assume process.nprocesses == 1 for the moment (eventually denominators[nprocesses]?)
---
>     const int denominators = 36, 36; // FIXME: assume process.nprocesses == 1 for the moment (eventually denominators[nprocesses]?)

diff pp_tt.auto/SubProcesses/P1_Sigma_sm_uux_ttx/CPPProcess.h pp_tt.auto/SubProcesses/P1_Sigma_sm_uux_ttx/CPPProcess.h.badfmt 
30,32c30,32
< // Process: c c~ > t t~ WEIGHTED<=2 @1
< // Process: d d~ > t t~ WEIGHTED<=2 @1
< // Process: s s~ > t t~ WEIGHTED<=2 @1
---
>   // Process: c c~ > t t~ WEIGHTED<=2 @1
>   // Process: d d~ > t t~ WEIGHTED<=2 @1
>   // Process: s s~ > t t~ WEIGHTED<=2 @1

diff uu_dd.auto/SubProcesses/P1_Sigma_sm_uux_ddx/CPPProcess.cc uu_dd.auto/SubProcesses/P1_Sigma_sm_uux_ddx/CPPProcess.cc.badfmt 
173,174c173,174
<       jamp_sv[0] += +1./6. * amp_sv[0];
<       jamp_sv[1] -= 1./2. * amp_sv[0];
---
>       jamp_sv[0] += +1. / 6. * amp_sv[0];
>       jamp_sv[1] -= 1. / 2. * amp_sv[0];

diff uu_tt.auto/SubProcesses/P1_Sigma_sm_uux_ttx/CPPProcess.cc uu_tt.auto/SubProcesses/P1_Sigma_sm_uux_ttx/CPPProcess.cc.badfmt 
178,179c178,179
<       jamp_sv[0] += +1./6. * amp_sv[0];
<       jamp_sv[1] -= 1./2. * amp_sv[0];
---
>       jamp_sv[0] += +1. / 6. * amp_sv[0];
>       jamp_sv[1] -= 1. / 2. * amp_sv[0];

diff heft_gg_h.auto/SubProcesses/P1_Sigma_heft_gg_h/CPPProcess.cc heft_gg_h.auto/SubProcesses/P1_Sigma_heft_gg_h/CPPProcess.cc.badfmt 
160d159
< 

valassi added 11 commits March 2, 2022 19:33
<       jamp_sv[0] += +1./6. * amp_sv[0];
<       jamp_sv[1] -= 1./2. * amp_sv[0];
---
>       jamp_sv[0] += 1. / 6. * amp_sv[0];
>       jamp_sv[1] -= 1. / 2. * amp_sv[0];
./tput/teeThroughputX.sh -inl -flt -makej -makeclean -eemumu -ggtt -ggttgg
STARTED AT Wed Mar  2 20:22:04 CET 2022
ENDED   AT Wed Mar  2 20:48:20 CET 2022
./tput/teeThroughputX.sh -flt -hrdonly -makej -makeclean -eemumu -ggtt -ggttg -ggttgg^C
STARTED AT Wed Mar  2 21:22:39 CET 2022
ENDED   AT Wed Mar  2 21:38:57 CET 2022
./tput/teeThroughputX.sh -flt -makej -makeclean -ggttg^C
./tput/teeThroughputX.sh -eemumu -ggtt -ggttgg -rmbhst
./tput/teeThroughputX.sh -eemumu -ggtt -ggttgg -bridge
./tput/teeThroughputX.sh -eemumu -curhst
./tput/teeThroughputX.sh -eemumu -common
@valassi valassi marked this pull request as ready for review March 3, 2022 06:44
@valassi valassi changed the title WIP: clang-format clang-format (and related code changes) Mar 3, 2022
@valassi
Copy link
Member Author

valassi commented Mar 3, 2022

Done:

This is now ready. I will self merge this.

valassi added 3 commits March 3, 2022 08:03
./tput/teeThroughputX.sh -flt -hrd -makej -makeclean -ggttggg
STARTED AT Wed Mar  2 22:27:19 CET 2022
ENDED   AT Thu Mar  3 00:44:23 CET 2022

Check all logs are updated:
grep DATE tput/logs_*/*txt | sort -k2
@valassi valassi merged commit 787f11f into madgraph5:master Mar 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant