Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Input requested: print of parameter( groups) #113

Open
krisgry opened this issue Jan 22, 2018 · 4 comments
Open

Input requested: print of parameter( groups) #113

krisgry opened this issue Jan 22, 2018 · 4 comments

Comments

@krisgry
Copy link
Contributor

krisgry commented Jan 22, 2018

Hi,
We have been discussing two parameter-related changes that we would like to have some input on:

  1. It is very useful to print parameters when they are changed, for timestamping user-input events (change of tuning parameters, activation of some feature, etc). A nice way to initialize this from the task would be something like:
    param("Awesome param").defaultValue("42").printOnChange(true, debugLevel="Debug")

  2. Another, related, change is to add the notion of a group of parameters; when changing the P-gain of a controller, it makes for less bookkeeping if the D- and I-gains are printed at the same time. Could be added by e.g. param("Awesome param").defaultValue("42").group("yawPID")

I know that it is possible to reconstruct the parameters from the setEntityParameter message, but it only tracks the changes, not the current status, which again calls for a lot of bookkeeping.

Both of these changes require changes to the core DUNE functionality, so I wanted to discuss them here first.

  1. The printing could be added in this if in the set-function in ParameterTable:
    if (parameter->commit())
    parameter->setChanged();

    by adding something like
    if (parameter.printOnChange && parameter.debuglevel == m_task->getDebugLevel() ).
    This will require the ParameterTable class to have a member variable that is a pointer to its class, to be able to print. One thing that is not clear to me yet is how the changes should be printed.
  • should the printOnChange function also take a print string as argument (like "%s was set to %f" when printing a float), or should this be detected from the type of the variable?
  • should the debug/spew/trace functions be called explicitly (to keep a clean interface), or just mimic the code:
    if (m_debug_level < DEBUG_LEVEL_DEBUG)
    return;
    std::va_list ap;
    va_start(ap, format);
    log(IMC::LogBookEntry::LBET_DEBUG, format, ap);
    va_end(ap);
  1. To add the group notion, one could add m_groups, which is a map from a string (group name) to a vector of parameters in that group. The parameter class would also need to add m_group. Then, after the above suggested if-statement, one would loop through all the parameters from the same group as the parameter, and print them all. Something like

group = m_groups.find(parameter.group); for(itr_group = group.second.begin(); itr_group != group.second.end(), ++itr_group) print

What do you think about this? As you notice, the changes are not major, but they are operating on some core DUNE classes. Very open for inputs before we start the implementation :) At this stage I have only looked through the code to plan the process, so there might be something that I have not considered.

@rbencatel
Copy link
Contributor

This sounds quite useful. Often I have felt the need for this kind of automatic outputs. Right now I program that directly on the the task, but this sounds like a much more efficient way to do it.

@josebraga
Copy link
Member

My two cents:

Do you need that output in a lot of files?

For one, SetEntityParameters, if logged, shows evolution of commands.

Second, if it is mainly for PID tuning why not add an appropriate, custom fitted debug string locally in onUpdateParameters? Your solution would see parameters being outputted with a standard format (unless you also pass a third argument with a format output string).. it seems too much of a hassle (to me) when you can just do that in the task.

Also, imagine your parameter needs to be converted to something else to be human readable (e.g to degrees), or it needs to be fed to a function first and you really want the output of that function? Your suggestion wouldn't help in these cases...

@mortenfyhn
Copy link
Contributor

I imagine using a task parameter vector might also give some of the functionality of parameter groups. (Such as param("PID Gains", m_args.pid_gains).defaultValue("0.0, 0.0, 0.0").size(3);.) I get the impression that this feature is used less than it should be.

@zepinto
Copy link
Member

zepinto commented Jul 12, 2018

Something related with this, we found out that when requesting the configuration of another task, the parameters do not get updated at runtime. You only get the initial (static) configuration from the .ini file. This is a big issue that we will be fixing soon and possibly this debug will help with that so thanks for your effort.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants