Thermal plotting additions

[valschneider]

This PR is about:

• Changing the drawstyle of plotting methods to 'steps-post'
• Adding cdev_state plotting methods for devfreq and cpufreq cooling devices.
• Adding xlim/ylim parameters to new and existing plotting methods (mostly to help in my next LISA PR)

This is what the plots look like: https://gist.github.com/valschneider/81a247b6acb69cccfe6e0ad7682914a8#file-thermal_plotting-ipynb

[valschneider]

@bjackman

• Fixed pydoc typos
• Added missing pydocs
• Used comp list
• Moved drawstyle to method parameters
• Added tests

Haven't had a look at factorizing the {cpu/dev}freq cdev state plotting code. I'll do that now.

[valschneider]

Okay so I think the best way to do this would be to merge the devfreq/cpu power classes, so that they may share logic. Their dataframes are quite similar, but:

• I don't know how much I should assume about those similarities
• That merging is gonna take some more time

[bjackman]

OK, if it's a big job then don't worry about it, the duplication isn't too bad.

[valschneider]

• Removed unused parameters title, ax and legend_label from cdev plotting methods
• Removed related pydoc
• Removed legend generation for cdev plotting methods since there's only one data set being drawn per plot.

[bjackman]

Other than the __init__ thing this LGTM. Can we leave it open for a little while though, in case anyone else gets time to review?

[valschneider]

Sure thing, I'd welcome some more feedback. I'll just fixup that __init__ thing.

[valschneider]

One more thing: I was thinking of adding plotting methods for the thermalpower{cpu/devfreq}_{get/limit} traces. Thing is, the thermal_power_allocator trace has the same data concatenated in a single trace BUT with mangled device names.

I was thinking of just reading the data from thermalpower{cpu/devfreq}_{get/limit} in LISA to populate the power_actors argument of thermal_power_allocator.plot_{in/out}put_power(), but it feels like it would be a cleaner job to have specific plotting methods in the devfreq/cpu related classes. Do you think that's worth to have ?

[valschneider]

@bjackman Added a plot_generic method in plot_utils.py. It allows me to factorize code for cpufreq/devfreq plotting, and also makes it easy to plot stuff on the go: https://gist.github.com/valschneider/89cbcc9318d80af74c4cc3f84cd8e075#file-thermal_plotting-ipynb

[derkling]

As an aside comment, there was a discussion in the past about moving plotting functionalities out of TRAPpy which should instead focus on trace parsing and DF generation.

Seems that in the past the main argument against this idea was backward compatibility.

Since here it seems we are at adding yet some other plots, I was wondering if we should not raise that question again and have another discussion about the opportunity to consolidate the tools we have around. To me the overall idea design should be:

• WA: data collection
• TRAPpy: data post-processing
• LISA: data analysis and this plotting

What do you think guys, what can be done to improve on this overall design?

[valschneider]

@derkling I was actually surprised to see there were plotting methods in TRAPpy- it's only for the thermal modules though.

Tbh, I believe it would be indeed cleaner to keep the plotting methods contained in LISA, but I also see the appeal of being able to easily plot stuff from within TRAPpyitself: you don't need yet another repository.

However, there's already a lot of plotting methods in LISA, and only thermal plots in TRAPpy. I'm not leaning strongly towards either choice, but I do believe either one should have plotting methods, not both.

[bjackman]

@derkling yeah these are good points, and I think your proposed split of responsibilities makes good sense.

The only problem I can see is that LISA cannot currently be used as a library (i.e. it uses the init_env thing) - it would be cool to be able to integrate LISA into other tools (e.g. as a results_processor in WA), which can already be done with TRAPpy. I did a little fiddling to see if that can be fixed, it seems like it definitely could but it would require reorganising the repository, and would also break backward compatibility because all imports would need to be changed (e.g. I think from trace import Trace would need to become from lisa.trace import Trace or similar). Do you know why we have the init_env thing?

[valschneider]

Closing this for now as after chatting with @derkling & @bjackman, it really makes more sense to have the plotting utilities in LISA.

[derkling]

@bjackman the init_env is there mainly because LISA tracks devlib and trappy as submodules and we want to be sure the local libraries are referenced. This was required at the beginning of the development when the LISA APIs was mostly dependant on leading-edge features of those libs. Actually, also more recently we usually merge LISA APIs which progress the dependencies as well... while those dependencies are not yet released as stable pip packages.

Finally, we use the init_env to setup the LISAShell which provides a set of custom convenience commands.

Another question is: do we really need to use LISA as a dependency of some other tool? In my view, LISA is that "umbrella" project which uses devlib, trappy and potentially WA... but none of these tools should be interested in using LISA as a dependency.

[bjackman]

(Repeating conclusion of a f2f discussion)

Agreed - my proposed use case for using LISA as a dependency of another tool is not really necessary, I think (or perhaps it could conceivably be useful but it is not worth the cost). Let's try to move plotting and analysis into LISA.

[bjackman]

Although.. saying that I've just remembered that TRAPpy has all the interactive plot stuff and "plotting" in the name! I suppose the proposal is to keep the generic stuff (ILinePlot & co) in TRAPpy and move out all the stuff that encodes knowledge about Linux itself?

[valschneider]

@bjackman I'll first move my PR & the other interesting plotting methods, and then we can see what remains in TRAPpy and what can be weeded.