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

[jiminy_py] Add option for plotter to compare several logfiles. #93

Merged
merged 6 commits into from May 4, 2020

Conversation

matthieuvigne
Copy link
Contributor

One of the main purpose I use jiminy for is to compare two simulations of the same system, with different parameters. In that context, I get two log files, with the same content (in terms of logged fields) and length.

This PR adds the capacity for jiminy_plotter to compare two or more logs, by plotting them on the same graph:

jiminy_plotter logfile.data regex -c the_logs_I_want_to_compare_to.data

Feature preview:

simu_jiminy_standstill_DTF_JointLQR data

@duburcqa
Copy link
Owner

duburcqa commented Apr 27, 2020

Nice feature !

But why not just jiminy_plotter logfile.data the_logs_I_want_to_compare_to.data ... regex ? Because it is not straigtforward to find the start and end of the regex I guess ? What you could do is to check if it is an existing path, or something similar.

@duburcqa
Copy link
Owner

duburcqa commented Apr 27, 2020

What do you think of being able to click of the legend on turn on/off the display of the logfiles ?
Just a (crazy) idea x) But it turns out to be pretty easy to implement in less than 10 lines of code !
https://stackoverflow.com/a/46550271/4820605

@matthieuvigne
Copy link
Contributor Author

I felt having a specific argument made the option clearer to the user (because it's documented by argparser). I would rather avoid mixing regex and logfiles, for clarity: that way you can understand the tool as taking

  • a (master) logfile
  • a list of plot commands, that can be some crazy regex
  • with -c, a list of logs to compare.

Otherwise I feel it's a bit more difficult to understand on what the regex apply: here you really give a specific role to the master logfile, and it's clearer to see -c as something you add on top of this. What do you think ?

I'll have a look at the interactive legend thing.

@duburcqa
Copy link
Owner

As you wish, I'm fine with it. It was just to better understand your point of view :)

resetProxies was not updating the model size, leading to issues when
switching from rigid to flexible model.
@matthieuvigne
Copy link
Contributor Author

@duburcqa Implemented interactive legend as you suggested.

@duburcqa
Copy link
Owner

duburcqa commented May 1, 2020

@duburcqa Implemented interactive legend as you suggested.

Sounds amazing ! Great work :)

@matthieuvigne
Copy link
Contributor Author

@duburcqa I've merge the other two PRs into this one. I've mutualized a bit the version number in the setup files, I can change back if you disagreee. I didn't look for a way to mutialize it between both packages, but this is already a start.

@matthieuvigne
Copy link
Contributor Author

I realized that the change I made is only python3-compatible, perhaps I should roll back.

From https://packaging.python.org/guides/single-sourcing-package-version/ , I found that setuptools_scm allows sourcing of the version from the git tag. This sounds like a nice feature on paper, but it practice it may be a bit confusing that the version is nowhere in the sources, only in the git tag gives the version (also, if the current commit is untagged / contains local modification, you get meaningless (but incremental) versions numbers like 1.2.32.dev0+gedb58a2a.d20200501)).

@duburcqa
Copy link
Owner

duburcqa commented May 2, 2020

I'm afraid the only robust way to do it is the Cmake configuration_file mechanism https://cmake.org/cmake/help/v3.2/command/configure_file.html.

Doing this, setting the version tag in the main cmake would be enough. And it is compatible with any Python version. I will do it soon.

About the dev branch, I think with should have one after all.

@duburcqa duburcqa force-pushed the plotter_compare branch 3 times, most recently from 185fc05 to 4a1ea96 Compare May 4, 2020 09:30
@duburcqa
Copy link
Owner

duburcqa commented May 4, 2020

Done ! Now there is a single entry point to set the version tag :)

@duburcqa duburcqa merged commit 9a0762a into duburcqa:master May 4, 2020
@matthieuvigne matthieuvigne mentioned this pull request May 12, 2020
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

Successfully merging this pull request may close these issues.

None yet

2 participants