Documentation and help shows restart with '--' instead of "+"

[clong83]

I found the struct in src/Control/Keywords.hpp, but I am unclear how to get this to show up as + instead of '--' when displayed for a single item, as that is templated in Base/print.hpp. SPecifically, it looks like Control/Inciter/CmdLine/Parser.cpp send the following prefix to print.hpp for all arguments on line 85-8:

if (argc == 1 || helpcmd) {
print.help< tk::QUIET >( tk::inciter_executable(),
cmdline.get< tag::cmdinfo >(),
"Command-line Parameters:", "-" );

I am not sure of an easy way to change that for only one or some of the members as that will get passed along. I could put in if check into the print.hpp and change the prefix if we are on the keyword 'restart'. Kinda hacky, but it should work.

I am going to prep for my meeting, but I'll return this later.

[jbakosi]

That piece of code was not designed to work with a '+'.

Every command line argument prefixed with a '+' is captured (intentionally) by the runtime system. This is how arguments to it are differentiated from those intended to the application.

Could you point to the documentation where restart is prefixed by a '--'? (I think that's wrong.)

[jbakosi]

Ah, I see it in the docs: https://quinoacomputing.github.io/inciter_cmd.html#inciter_cmd_kw_restart

That is wrong and (I believe) not used. So I would suggest removing this code from src/Control/Keywords.hpp:

struct restart_info {
  static std::string name() { return "checkpoint/restart directory name"; }
  static std::string shortDescription()
    { return "Specify the directory for restart files"; }
  static std::string longDescription() { return
    R"(This option is used to specify the directory name in which to save
    checkpoint/restart files.)";
  }
  using alias = Alias< R >;
  struct expect {
    using type = std::string;
    static std::string description() { return "string"; }
  };
};
using restart = keyword< restart_info, TAOCPP_PEGTL_STRING("restart") >;

[clong83]

Ah, I see it in the docs: https://quinoacomputing.github.io/inciter_cmd.html#inciter_cmd_kw_restart

That is wrong and (I believe) not used. So I would suggest removing this code from src/Control/Keywords.hpp:

struct restart_info {
static std::string name() { return "checkpoint/restart directory name"; }
static std::string shortDescription()
{ return "Specify the directory for restart files"; }
static std::string longDescription() { return
R"(This option is used to specify the directory name in which to save
checkpoint/restart files.)";
}
using alias = Alias< R >;
struct expect {
using type = std::string;
static std::string description() { return "string"; }
};
};
using restart = keyword< restart_info, TAOCPP_PEGTL_STRING("restart") >;

Yes, we had a user try to restart this way (and cited me this from the documentation) and lost their previous simulation, so I wanted to get it fixed.

You are right that it might be best to just delete the "restart" bit entirely and include better documentation of how to use restart elsewhere on the site. Making the restart one alone show up with a "+" might be more confusing than it is worth, and is not entirely accurate because that is a runtime argument and not an inciter argument.

[jbakosi]

This is because restarts are handled by the runtime system. Inciter only specifies what data needs to be saved, but the runtime system does the actual saving of the checkpoint and the restart from it.

[adityakpandare]

I have added some clarification on our doc-page (fa4a9b4). Obviously does not fix the entry in the Command line arguments page.

[jbakosi]

This is useful.

What about removing the code I suggest in #637 (comment)? If that's done, I believe regenerating the docs should also remove the misleading doc entry.

[clong83]

There's a problem with that because the argument is actually used. The user can specify a specific name for the restart folder using it, and when a restart is initiated using +restart, the argument gets passed to inciter somehow under the hood and stored in that container.

Long story short, deleting that code breaks restarts.