Replace custom progress with cleaner tqdm

[juhannc]

tqdm is a very clean and fast way to display the progress of a loop. Just as the currently used custom progress prints, it displays the progress in percent as well as the elapsed and total number of timesteps, the elapsed and remaining time and the time per iteration.

Unless the plan was to display further information in the progress, I find tqdm to be way cleaner. Also, I think all other relevant performance metrics are better logged into Tensorboard and not the shell. But feel free to reject the PR if you had some more stuff planned for the progress display.

[Toni-SM]

Hi @JohannLange,

It is a good idea to change the way progress is displayed... however, the pull request is a bit unclean. Please, open a new pull request only with the minimum necessary modification for integrating tqdm as described below:

• modified files: trainer/torch/base.py, trainer/torch/sequential.py, trainer/torch/paraller.py and setup.py
• don't modify whitespace, blank lines, etc. Those cosmetic changes do not add anything substantial to the library's functionality.
• add import statements as follow:

  function annotation
  <empty line>
  python libraries and other libraries (e.g. tqdm)
  <empty line>
  machine learning framework (e.g. torch)
  <empty line>
  skrl components

  One question, have you tested the changes with the parallel trainer? Multiprocessing implementation may not be compatible with some modules containing anonymous functions or locks.

Btw, if you are interested in making further contributions, I have a list of future relevant implementations and pending tasks... it can be an interesting starting point :)

[juhannc]

Hi @Toni-SM,

thank you for your feedback! Nevertheless, I have some questions/comments we should discuss before merging the PR.

• modified files: trainer/torch/base.py, trainer/torch/sequential.py, trainer/torch/paraller.py and setup.py

First things first, sorry for re-adding the GitHub workflow, that was by mistake. But regarding the examples in the docs, I updated them and removed the "progress_interval" setting, because it will be deprecated if you decide to merge the PR.

• don't modify whitespace, blank lines, etc. Those cosmetic changes do not add anything substantial to the library's functionality.

Well here we both have a clearly different opinion. For me removing trailing whitespaces mainly comes down to two reasons based on my comfort, besides most IDEs removing them anyway, first, it makes navigating with keyboard commands unnecessarily hard, and second, it creates a lot of "noise" with activated white space symbols like interpunct for spaces, right arrow for tabs and the pilcrow.

Also, while removing the trailing spaces doesn't add anything to skrl, neither does keeping them.

And lastly, pep 8 recommends removing them anyway, best with a pre-commit hook, the later I could easily add if you wish to.

Oh and also that empty line at the end of file is really something that should be there, see stackoverflow and unix.stackexchange. Basically, it boils down to an old decision in ANSI C 1989 and was later incorporated into the POSIX standard

• add import statements as follow:

  function annotation
  <empty line>
  python libraries and other libraries (e.g. tqdm)
  <empty line>
  machine learning framework (e.g. torch)
  <empty line>
  skrl components

That I can live with, I will reorder the import where I distorted the order and push a new commit.

One question, have you tested the changes with the parallel trainer? Multiprocessing implementation may not be compatible with some modules containing anonymous functions or locks.

I tested it for gym and Isaac Gym and it worked just fine. Although I couldn't test it for Omniverse Gym/Sim because I don't use that, but I would suspect it behaves the same. I hope that answers your question.

Btw, if you are interested in making further contributions, I have a list of future relevant implementations and pending tasks... it can be an interesting starting point :)

Yes! I would happily help as I plan to further continue using skrl for its ease of configuration!

[Toni-SM]

Yes! I would happily help as I plan to further continue using skrl for its ease of configuration!

Glad to hear about it... Here (https://github.com/users/Toni-SM/projects/2/views/8) is the pending tasks board in case you want to take a look