search

EngineerDanny / Rperform

:bar_chart: R package for tracking performance metrics across git versions and branches.
https://analyticalmonk.github.io/Rperform
GNU General Public License v3.0
1 stars 0 forks source link

gglot2 graph plot issue. #4

Closed EngineerDanny closed 2 years ago

EngineerDanny commented 2 years ago

After, running the Rperform::plot_branchmetrics(test_path = "tests/testthat/test-CRAN.R", metric = "time", branch1 = "l1loss", branch2 = "master", save_data = T, save_plots = F) function on the binsegRcpp repository, comparing the master and l1loss branch. I recorded a time_frame of 254 observations and 8 variables. Below is how the graph looks like :

Rplot

You could see that information is not depicted clearly on the graph. My hypothesis is that it was caused as a result of the larger data set. Potential fix will be breaking down the data sets and plotting their respective graphs.

tdhock commented 2 years ago

thanks for sharing. not sure what the problem with that plot is.

  1. can you share the files which were generated?
  2. why are there only two commits on the X axis? there should be several dozen in that branch.
EngineerDanny commented 2 years ago

This was the data set generated when the function was ran. 👉 time_frame.csv

Yes, there are actually several commits on that branch. But the plot_branchmetrics function works differently as explained here.

Master : It only tests the latest commit in this branch .In this case, it is Merge pull request... commit.

PR Branch : It tests all the commits in this branch after the latest common commit among the two branches. In this case, it is checks ok commit.

This explains why there are only two commits on the X-axis.

EngineerDanny commented 2 years ago

This is how the commit history looks for the master and l1loss branches respectively.

Screenshot 2022-06-24 at 11 09 37 AM Screenshot 2022-06-24 at 11 09 17 AM
tdhock commented 2 years ago

there were 42 commits in the corresponding PR, which was already merged with master. https://github.com/tdhock/binsegRcpp/pull/10/commits maybe you need to refer to the explicit commit SHA1 IDs to get a time series of 42 commits?

EngineerDanny commented 2 years ago

Yh. It looks like it squashed the 42 commits into just one merge request commit and used that.

EngineerDanny commented 2 years ago

@tdhock I have been looking into this, when the test_names are many. This means there are lots of testthat blocks in just one file. Hence a longer graph output.

ggsave1

EngineerDanny commented 2 years ago

@tdhock The new update exposes vital parameters like the height_in_inches and width_in_inches. Do you think I should expose res too ?

tdhock commented 2 years ago

yes that could be useful