Skip to content

Reducing the content of patchmad.sh #756

Open
Open
Reducing the content of patchmad.sh#756

Description

@oliviermattelaer
oliviermattelaer
opened on Aug 23, 2023

I'm listing here all the content/step of that script and what I think should be the correct handling:

  • run treatcards (param/run)
    • SUGGESTION OM: do not run those at output command (only relevant at running time)
  • run some cleaning
    • SUGGESTION OM: those are in the wrong file, should not be run in the output command (maybe update clean.sh script ?)
  • Add global flag '-O3 -ffast-math -fbounds-check' as in previous gridpacks
    • SUGGESTION OM: This should be handle via the run_card (and ffast-math is too dangerous at that level)
    • SUGGESTION OM (in addition) , we can change default value for those flag when the plugin is activated
  • adding fbridge_common.inc to the code
    • SUGGESTION OM: this should be done during the export (so in output.py)
  • change sde_strategy (enforce 1)
    • SUGGESTION OM: this should be handle by the run_card and not here
    • SUGGESTION OM (in addition): we should add a check that the parameter is always 1
  • change f2py compiler (why?)
    • SUGGESTION OM: this should not be done at all, this is controlled by the configuration file of MG5aMC (user specific)
  • change to coupl_write.inc (not sure to get the problem/point here)
    • SUGGESTION OM: should be done in output.py (can keep it like this is a first stage)
  • apply real patch file .common
    • SUGGESTION OM: should be done in output.py (can keep it like this is a first stage)
    • SUGGESTION OM: File make_opts should not be touched (at least the part which is related to the user choice)
      • patch is actually rejected on my machine due to machine specific option
  • apply real patch file .P1
    • SUGGESTION OM: should be done in output.py (can keep it like this is a first stage)

Additionally many of those command are os sensitive (cp use "version dependent option" as well as sed")

  • remove option "-d" for the cp
  • avoid the use of the sed option

Activity

  1. oliviermattelaer commented on Aug 30, 2023

    @oliviermattelaer
    oliviermattelaer
    on Aug 30, 2023
    MemberAuthor

    python file modify by patch (create issue with launch)

    • banner.py (no need)
      • add parameter in the run_card (fixed via generic method)
    • gen_ximprove.py
      • change compile code -> compile function now catched by the interface in launch_plugin.py
    • madevent_interface.py
      • change compile code -> compile function now catched by the interface in launch_plugin.py

  2. oliviermattelaer commented on Feb 2, 2024

    @oliviermattelaer
    oliviermattelaer
    on Feb 2, 2024
    MemberAuthor

    In #813
    I have remove the non standard handling of

    • fbridge_common.inc
    • counters.cc
    • ompnumthread.cc
      (all move to the appropriate class variable of output.py)

  3. oliviermattelaer commented on May 23, 2024

    @oliviermattelaer
    oliviermattelaer
    on May 23, 2024
    MemberAuthor

    Note that #849 removes the patch to the file, which are not needed since the compile function is anyway handle correctly without such patch. (since the merge of the new_interface branch and in particular of this commit: 5b22a92)

    @valassi , any (apriori) issue if I implement the suggestions above? I will do a PR anyway (but if you have some of the above suggestion on which you do not aggree then better to do the un-controversial first and then think more about the other ones)

  4. oliviermattelaer commented on May 23, 2024

    @oliviermattelaer
    oliviermattelaer
    on May 23, 2024
    MemberAuthor

    I have updated the above list of comment since looks like the situation did improve (a lot) but was not reported above.
    So most of this has been implemented but low priority stuff:

    So the "open" question remaining are

    • default value for compiler flag for the moment the default is
      aloha_flag='--fast-math' global_flag='' matrix_flag='-O3'
      maybe better to move to
      aloha_flag='--fast-math -O3' global_flag='' matrix_flag='-O3'

    • coupl_write.inc, this one seems quite complex and I do not really understand what the patch does...

     71 # Patch the default Fortran code to provide the integration with the cudacpp plugin
  72 # (2) Process-dependent patches
  73 cd ${dir}/Source/MODEL > /dev/null
  74 gcs=$(cat coupl_write.inc | awk '{if($1=="WRITE(*,2)") print $NF}') # different printouts for scalar/vector couplings #456
  75 for gc in $gcs; do
  76   if grep -q "$gc(VECSIZE_MEMMAX)" coupl.inc; then
  77     ###echo "DEBUG: Coupling $gc is a vector"
  78     cat coupl_write.inc | awk -vgc=$gc '{if($1=="WRITE(*,2)" && $NF==gc) print $0"(1)"; else print $0}' > coupl_write.inc.new
  79     \mv coupl_write.inc.new coupl_write.inc
  80   ###else
  81   ###  echo "DEBUG: Coupling $gc is a scalar"
  82   fi
  83 done
  84 cd - > /dev/null

    This sounds linked to some quite complex handling of the model that you do handle, it is not a priority to change this for sure but wonder if I can make you life easier if I handle this at the python level. Tell me what you think on this and will see if it is worth to try to move this on the python logic...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions