NikolausDemmel / rootba

Square Root Bundle Adjustment for Large-Scale Reconstruction
BSD 3-Clause "New" or "Revised" License
279 stars 38 forks source link

CANNOT add more cli paramters #7

Closed BayRanger closed 2 years ago

BayRanger commented 2 years ago

Hi, I've been trying to add another cli paramter to set groundtruth file for system benmarking. But it cannot work.

  VISITABLE_META(std::string, input, help("input dataset file to load"));
  VISITABLE_META(std::string, ground_truth, help("input ground_truth file to load"));
  VISITABLE_META(DatasetType, input_type,
                 init(DatasetType::AUTO).help("type of dataset to load"));

It's mentioned that I have to add it to docstring, so I add it to configuration.md also. Still, it cannot work. After some debuging, it returned false in following code. But I don't understand why it failed.

  // parse arguments
  if (!parse(argc, argv, cli)) {
    auto executable_name = std::filesystem::path(argv[0]).filename();
    auto fmt = doc_formatting{}.doc_column(22);
    auto filter = param_filter{}.has_doc(tri::either);
    if (!application_summary.empty()) {
      std::cout << application_summary << "\n\n";
    }

    std::cout<<"ffffff\n";
    std::cout << "SYNOPSIS:\n"
              << usage_lines(cli, executable_name) << "\n\n"
              << "OPTIONS:\n"
              << documentation(cli, fmt, filter) << '\n';
    return false;
  }

So could you give me some hints about how to make it work?

NikolausDemmel commented 2 years ago

What is the error you see? Compile time or runtime?

If compile error, please provide the full compiler output and the compiler version and OS you are using.

If runtime, please provide the full command line you are running and the full console output including the error.

BayRanger commented 2 years ago

It could compile, but the added parameter "ground_truth" cannot be recognized.(I could hard code it in the bal.cpp and it works) So it made me confused about how to add a new cli parameter. The ouput is shown as

Solve BAL problem with solver determined by config.

ffffff
SYNOPSIS:
        bal [-C <DIR>] [--config <PATH>] [--dump-config]  [--input <STR>] [--groundtruth <STR>]
            [--input-type <ENUM>] [--save-output|--no-save-output] [--output-optimized-path <STR>]
            [--normalize|--no-normalize] [--normalization-scale <FLOAT>] [--rotation-sigma <FLOAT>]
            [--translation-sigma <FLOAT>] [--point-sigma <FLOAT>] [--random-seed <INT>]
            [--init-depth-threshold <FLOAT>] [--quiet|--no-quiet]  [--solver-type <ENUM>]
            [--verbosity-level <INT>] [--debug|--no-debug] [--num-threads <INT>]
            [--residual-robust-norm <ENUM>] [--residual-huber-parameter <FLOAT>] [--log-log-path
            <STR>] [--log-save-log-flags <FLAG>...] [--log-disable-all|--no-log-disable-all]
            [--optimized-cost <ENUM>] [--max-num-iterations <INT>] [--min-relative-decrease <FLOAT>]
BayRanger commented 2 years ago

BTW. command line is as following ./bin/bal --input 'github/rootba/data/simulation/simulation_data.txt' --ground_truth 'github/rootba/data/simulation/simulation_ground_truth.txt'

BayRanger commented 2 years ago

Well, finally I figured it out, the parameter I defined named 'ground_truth', but the docstring seems to automatically convert it to 'ground-truth' and made it not work. My solution is to rename it as 'groundtruth'.

NikolausDemmel commented 2 years ago

Great to hear it works now.

For future reference: To be more idiomatic, underscore _ is automatically converted to dash - for the command line. So --ground-truth should have worked. (The conversion happens here: https://github.com/NikolausDemmel/rootba/blob/0762f36a0afa7196709bd0fa147ae75ee7c7632c/src/rootba/cli/cli_options.cpp#L61)

BayRanger commented 2 years ago

Thank you for your quick reply, this cli conversion made me really impressive, so what is the motivation to do this conversion from '-' to '_'?

NikolausDemmel commented 2 years ago

It's just that to me for the command line using - is more natural and common and --foo_bar looks strange compared to --foo-bar. Pure personal preference, no deeper meaning ;-). Sorry for the confusion.