Unknown parameter f77 formatting issue

[Zachary Marshall]

Hello authors, and happy new year!

We've identified certain situations when parameters are included in a run card but are not known in banner.py. When the translation is done from run_card.dat into run_card.inc, these parameters have type internal.banner.UnknownType, and the automatic fortran formatting decides that they are strings almost independent of their value. Here's an example of the issue:

    STR_INCLUDE_PDF = 'True '
    STR_INCLUDE_FLUX = 'True '

while the correctly generated code is:

    STR_INCLUDE_PDF = .TRUE.
    STR_INCLUDE_FLUX = .TRUE.

A small patch to banner.py is sufficient to solve this problem for us:

2642,2643d2641
< if value.strip() in ['1','T','.true.','True','0','F','.false','False']:
< formatv = 'bool'
2651c2649
< if str(value).strip() in ['1','T','.true.','True']:
---
> if str(value) in ['1','T','.true.','True']:

Notice the addition of strip, since frequently 'value' picks up white space. I've mirrored the values used for True as possible values for False here. This particular patch is against 2.9.9; in the current trunk of MG the equivalent patch would be L2998 and 3006 of banner.py.

Would you be so kind as to include this in the two branches going forward?

Thank you for your help,
Zach

PS - Thanks to Daniele Zanzi for the original report, and to Marco Zaro for helping with some of the debugging.

[Olivier Mattelaer (olivier-mattelaer)]

Hi,

This is safe patch.

First, the fact that you assume that 0 and 1 need to be translate to a boolean does not make sense to me.

Second, while changing the behavior for undefined entry in a card is certainly an option for our 3.x branch.

I do not see this as a bug fix but as a new feature and a change of behavior.
Additionally, supporting only bool, does not make sense. So if we do that, we should also consider
- support all types
- add the automatic definition of such variable (i.e. the line "bool str_include_pdf")
- have syntax to force the type based on the named (i.e. if a name start with str_ it will be a string, bool_ will be a bool/....) and automatically remove the prefix in that case.

So this is certainly not something that I see as compatible with the LTS branch since this is "debug only" and this is clearly a new feature with side effects that can impact PLUGIN/patch/...

< if str(value).strip() in ['1','T','.true.','True']:
---
> if str(value) in ['1','T','.true.','True']:

This change is less controversial to me and can be targeted for LTS if needed (but does it helps you?)

Cheers,

Olivier

> On 4 Jan 2023, at 07:12, Zachary Marshall <email address hidden> wrote:
>
> Public bug reported:
>
> Hello authors, and happy new year!
>
> We've identified certain situations when parameters are included in a
> run card but are not known in banner.py. When the translation is done
> from run_card.dat into run_card.inc, these parameters have type
> internal.banner.UnknownType, and the automatic fortran formatting
> decides that they are strings almost independent of their value. Here's
> an example of the issue:
>
> STR_INCLUDE_PDF = 'True '
> STR_INCLUDE_FLUX = 'True '
>
> while the correctly generated code is:
>
> STR_INCLUDE_PDF = .TRUE.
> STR_INCLUDE_FLUX = .TRUE.
>
> A small patch to banner.py is sufficient to solve this problem for us:
>
> 2642,2643d2641
> < if value.strip() in ['1','T','.true.','True','0','F','.false','False']:
> < formatv = 'bool'
> 2651c2649
> < if str(value).strip() in ['1','T','.true.','True']:
> ---
>> if str(value) in ['1','T','.true.','True']:
>
> Notice the addition of strip, since frequently 'value' picks up white
> space. I've mirrored the values used for True as possible values for
> False here. This particular patch is against 2.9.9; in the current trunk
> of MG the equivalent patch would be L2998 and 3006 of banner.py.
>
> Would you be so kind as to include this in the two branches going
> forward?
>
> Thank you for your help,
> Zach
>
> PS - Thanks to Daniele Zanzi for the original report, and to Marco Zaro
> for helping with some of the debugging.
>
> ** Affects: mg5amcnlo
> Importance: Undecided
> Status: New
>
> --
> You received this bug notification because you are subscribed to
> MadGraph5_aMC@NLO.
> https://bugs.launchpad.net/bugs/2001583
>
> Title:
> Unknown parameter f77 formatting issue
>
> Status in MadGraph5_aMC@NLO:
> New
>
> Bug description:
> Hello authors, and happy new year!
>
> We've identified certain situations when parameters are included in...

[Olivier Mattelaer (olivier-mattelaer)]

I have started a branch to add such support.
Here is where I'm currently tracking the status:
https://github.com/mg5amcnlo/mg5amcnlo/pull/27

As you will see, the auto-detection is present but not, yet the automatic definition inside one include (but the framework to decide which one is already present).
Note that the detection is done at python level and not the time when writting the include file, so the python representation is on sync, which is much nicer and generic.

Do not hesitate to comment on the syntax/... either here or on that thread.

Cheers,

Olivier

[Olivier Mattelaer (olivier-mattelaer)]

Implementation of the new features for 3.5.0 is complete.
Merge request is in progress.

[Zachary Marshall (zach-marshall)]

This is a lot of work that you've done, thank you Olivier! I think some of the added functionality is very interesting, and could become quite useful in the future. I agree with what you say about enforcing better naming conventions for additional parameters as well (these particular ones came from the MadSTR plugin). I understand your concern about adding this development to 2.9.9, and I'm happy with having this all sorted out in the development version.

Best,
Zach

[Olivier Mattelaer (olivier-mattelaer)]

For MadSTR, the cleanest is likely that MadSTR plugin adds those parameters as
expected parameters that allows madstr to define a default value, a type and ensure that the parameters is always defined, ...

I will discuss this with Marco.

> This is a lot of work that you've done, thank you Olivier! I think some
> of the added functionality is very interesting, and could become quite
> useful in the future.

I hope it will.

Cheers,

Olivier

> On 8 Jan 2023, at 21:03, Zachary Marshall <email address hidden> wrote:
>
> This is a lot of work that you've done, thank you Olivier! I think some
> of the added functionality is very interesting, and could become quite
> useful in the future. I agree with what you say about enforcing better
> naming conventions for additional parameters as well (these particular
> ones came from the MadSTR plugin). I understand your concern about
> adding this development to 2.9.9, and I'm happy with having this all
> sorted out in the development version.
>
> Best,
> Zach
>
> --
> You received this bug notification because you are subscribed to
> MadGraph5_aMC@NLO.
> https://bugs.launchpad.net/bugs/2001583
>
> Title:
> Unknown parameter f77 formatting issue
>
> Status in MadGraph5_aMC@NLO:
> Fix Committed
>
> Bug description:
> Hello authors, and happy new year!
>
> We've identified certain situations when parameters are included in a
> run card but are not known in banner.py. When the translation is done
> from run_card.dat into run_card.inc, these parameters have type
> internal.banner.UnknownType, and the automatic fortran formatting
> decides that they are strings almost independent of their value.
> Here's an example of the issue:
>
> STR_INCLUDE_PDF = 'True '
> STR_INCLUDE_FLUX = 'True '
>
> while the correctly generated code is:
>
> STR_INCLUDE_PDF = .TRUE.
> STR_INCLUDE_FLUX = .TRUE.
>
> A small patch to banner.py is sufficient to solve this problem for us:
>
> 2642,2643d2641
> < if value.strip() in ['1','T','.true.','True','0','F','.false','False']:
> < formatv = 'bool'
> 2651c2649
> < if str(value).strip() in ['1','T','.true.','True']:
> ---
>> if str(value) in ['1','T','.true.','True']:
>
> Notice the addition of strip, since frequently 'value' picks up white
> space. I've mirrored the values used for True as possible values for
> False here. This particular patch is against 2.9.9; in the current
> trunk of MG the equivalent patch would be L2998 and 3006 of banner.py.
>
> Would you be so kind as to include this in the two branches going
> forward?
>
> Thank you for your help,
> Zach
>
> PS - Thanks to Daniele Zanzi for the original report, and to Marco
> Zaro for helping with some of the debugging.
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/mg5amcnlo/+bug/2001583/+subscriptions
>

[Olivier Mattelaer (olivier-mattelaer)]

Hi,

I have discussed with Marco today, and he is actually using the "add_param" functionality to define such parameters to boolean.

However, to have it working you can not run the code from
./bin/mg5 or ./bin/mg5_aMC
executable
but you have to use the script from the generated directory
./bin/generate_events (or ./bin/aMCatNLO)

The MadSTR binary does prevent you to run the launch command.
But if you do "./bin/mg5_aMC launch MADSTR_DIR"
MG5aMC will think that MADSTR_DIR is a "normal" aMC@NLO directory and will not prevent you
to run it, but it will crash because of the above parsing issue.

So the plan is to add both in LTS and in 3.x version, a check within MG5aMC that separate normal aMC@NLO output from madstr output to ensure that the second type crashed nicely.