Comment 1 for bug 2001583

Revision history for this message
Olivier Mattelaer (olivier-mattelaer) wrote : Re: [Bug 2001583] [NEW] Unknown parameter f77 formatting issue

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 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
>