Code review comment for lp:~maddevelopers/mg5amcnlo/plugin_mode

Revision history for this message
Olivier Mattelaer (olivier-mattelaer) wrote :

Thanks a lot for this review

> a) For the interface plugin it would be cool if the prompt reflected which
> plugin the user is in (actually the prompt could be an attribute of the
> plugin, and if absent it is by default its name).

sounds too complicated.

> Maybe also mention on the wiki that people can define their own functions
> 'complete_<command_name>' and 'help_<command_name>' as well as putting them in
> the example helloworld.

Done

> b) For the output plugin, it would be nice if the user could also redefine
> what 'launch' does. For now the plugin user simply cannot launch on his own
> output.

Hum for the moment, I would see that as part of the interface plugin.
I also guess that they are less interest since the idea is typically to run externally to MGaMC

> d) Suggestion: It would be interesting to see if you could also automatically
> "install" a plugin from a repository somewhere.
> So basically implement the command 'install --plugin=<plugin_name>' in
> madgraph_interface.
> This command would look up the repository if the folder '<plugin_name>' exists
> on the repo and copy it locally in PLUGIN if it does.

I agree on the interest. So far no one really show interest in that direction.
If this happens then yes this can be done.

> e) You should maybe update the wiki page so as to describe the basic plugin
> 'user_filter.py'.

This is not really a plugin.
I have create another page for it (under the FAQ of the wiki)
https://cp3.irmp.ucl.ac.be/projects/madgraph/wiki/FAQ-General-15

> 2) What is the new 'partial_save' of the MG5aMC options?

Now you can do command like
save options collier
and only collier value will be modified in the mg5_configuration.txt

> 3) Could we have the plugin user_filter also work for the loops? I can help
> with this.

This should be trivial.
But with the level of your loop_filter, I guess that you will want something more refined than my dummy method. If you need help tell me.

> Anyway, none of the above is crucial and the unit tests pass (acceptance
> running now, but so far they're fine).
>
> I therefore accept the merge.

Great thanks.

Cheers,

Olivier

PS: Marco do you plan to review this branch? If not I plan to merge it on Monday. I can obviously wait if you want to take a look.

« Back to merge proposal