Use new `memory_stats` call; use pandas for formatting

[andyljones]

This PR

• Adds detailed memory stats
• Handles peaks using reset_peak_memory_stats() rather than emptying the cache
• Dumps the trace as a pandas dataframe, which can be used for downstream analysis
• Does the formatting using pandas' to_string, so variable numbers of fields are supported

Upshot is that .records() will give you a dataframe like this

with 96 columns covering every memory stat Torch offers. Terminal output can be whatever subset you want:

I don't think you'll want to take this PR as is; I've broken a bunch of stuff getting to what I need for the task at hand. I'm more making this PR in case you want to nab parts for the main branch.

[andyljones]

now with bonus HTML barcharts

[andyljones]

And now it accurately reports peaks and allocated/freed when you're profiling a hierarchy of functions.

This has been really useful for me, thanks so much for making it!

I've tried to restore some of the things I know that I broke, but some other stuff is still going to block merging with main. In particular,

• It only does HTML output right now, no text
• It requires pandas as a dependency. This might be too heavy for your tastes.
• It won't work on 2.7, I wasn't really paying attention to 2-friendly syntax.

[Stonesjtu]

It's great that you liked this tool. This is really a big PR, hopefully I can review these changes thoroughly this weekend.

[andyljones]

Thanks! No need to feel rushed though, and I understand if you decide it's too far from the original tool to be justifiable to fold in.

Really it's the pandas dep that's the critical bit: if you're okay with it, I'm happy to do the minimal extra work to get the tests working. Since pandas is a big library and doesn't support 2.7 any more, that might be too much for you though.

[Stonesjtu]

@andyljones I'm totally ok to drop python2.7 support. We have only < 1% python2.7 downloads.

Pandas dependency should not be a problem for me, it's a pretty common package. But I'm wondering if we can bypass pandas for the command-line interface.

[andyljones]

Ok, I've resolved lots of the smaller issues. Bigger ones:

• Text output: IPython handles objects that could be either text or HTML by looking for specific methods on the object returned by the cell. This means we can definitely do text output, but it'll require returning an object from print_stats, which means print_stats won't be able to take a stream. What might be better is to move all the rich output to a new .render()/.display() method which'll return an object, and print_stats can pipe text to whatever stream it's been passed. That has the advantage of preserving the existing API.
• Terminal pandas: I'm inclined to use pandas to format the text output too. That's what I used for the text styling at the top of the thread: and I think implementing a pandas-free path for gathering the data and then formatting variable numbers of columns with multi-line headers would be pretty verbose and painful.

[Stonesjtu]

Let's clean things up based on ipython and pandas

[andyljones]

Great. I'll do a full pass. Rather than list off individual names you'd like changed, is a there a style guide you can point me at?

[Stonesjtu]

We followed Google's python style guide.

[andyljones]

Right:

• Now does text or html output
• Added docstrings
• Added a global interpreter reset cause @profile accumulates in an ugly way in an interpreter session
• Got the tests working
• Got travis working
• Updated the readme
• Did lots of small fixes

[andyljones]

can you make the function name an action rather than a thing? e.g. _subset_line_records to _extract_line_records, _line_records_merged_with_code to merge_line_records_with_code.

I've done this, but as an aside this is an idiom from functional programming. If a function has no side-effects, it effectively is its return value, so it should be called a noun rather than a verb. Think of it as soft command-query separation.

OK. I've fulfilled most of the revision requests, and at this point the cleanup for the PR has taken substantially longer than implementing the original functionality. I respect that you've got high standards for the code in your projects, and I know you've invested time too in reviewing my code, but I'm afraid I can't put any more time towards this. If you're not happy with the PR as-is, and you don't want to make the final revisions yourself, that's okay: I'll close this and I'll instead publish the functionality I wrote as it's own package. I'd be disappointed, but unfortunately that's just how things work out sometimes.

[Stonesjtu]

Andy, thanks for your valued time to make this PR and address my suggestions. This PR has been impressive since the first proposal. I'm just trying to help you make it more pythonic and readable (for most python coders). It's a common situation that people spend more time addressing code reviews than functionality.

I can merge this PR first and then do some changes myself, but I think it's better for you to make this PR a complete one. Sadly you don't have that amount of time.

[andyljones]

Huurah! Thanks very much.

I think this might be a workplace culture thing; my previous experience has been much more relaxed about code style (as you can likely tell!). It's my bad for not asking after your standards originally, but you might also want to add some brief contributor guidelines to the readme or wiki.