Can we remove some of the bookkeeping modules?

[john-science]

The "bookkeeping" module has some important database tools in it, but it also feels like a grab bag of random stuff. Much of it isn't really used in ARMI, and most of it seems to have been written ad-hoc with little-to-no tests.

Which of the following modules is used, and for what? (If it is important, great, but I just want to know before I spend serious time testing and improving this code.)

• armi/bookkeeping
  • historyTracker.py (60% -> now 82%) - Why is the use-case for this?
  • memoryProfiler.py (50% -> now 85%)
  • db/
    • database3.py (88%)
    • compareDB3.py (25% -> now 83%)
  • report/
    • data.py (55%)
    • reportingUtils.py (71%)
    • newReports.py (90%) - How is this different from the report module?
    • newReportUtils.py (95%)

[john-science]

@keckler Here is a starting point for our discussion.

@ntouran We'd love your input here.

[john-science]

Ideas, upon talking to Nick:

• I think the memoryProfiler is hard-wired into operator.py or something. Can we make this an optionally importable? Maybe this could be moved to utils?
• We want to transition to newReports, but is anyone using it yet? CAN we transition?
  • The problem here is staff left during the transition.
• compareDB3.py is used to compare the outputs of runs.

[ntouran]

• Yeah memoryProfiler is more of a utils-type of thing. In fact there are lots of memory related utils already in utils. We have an actual memoryProfiler interface in the bookkeeping module that fires of memory reports at various interaction hooks. Since it's an actual interface, it ended up in the bookkeeping subpackage alongside the other related interfaces. If we want to move the memoryProfilerInterface into utils, then so be it. Long ago we had all things that were interfaces in a special subpackage but we've left that convention now.

• we should write some roadmapping related to reports. It would indeed be ideal to phase one out and the other in. In any case they should probably both be in the report subpackage. Neither report system has too many users as far as I'm aware.

• compareDB3 is essential for comparing two output databases. It's a pivotal part of any application's in-use test suite, and is also useful for doing sensitivity studies (e.g. change a cross section and see how the entire state vector changes). I think it's appropriate in there and should have more unit testing.

• plotting could be in utils for sure. Just intended to be some convenient re-usable plotting code related to the data model.

[john-science]

I have seriously cleaned up memory_profiler.py. It was clearly not used since Python 2.x. There were also bugs where it referenced ARMI methods that no longer existed. Also, it's use of pympler made it roughly 500 times slower than the standard library. So I have it all working again.

Also, I removed some unused/broken code, and increased the code coverage.

[john-science]

I have uncovered that some of the cruft in the bookkeeping module exists because there was a re-write in progress that was dropped when people left the project:

1. We have too many reporting tools, because there was a transition that was never completed: Clean up the Reporting Tools #793
2. The History Tracker Interface could probably be removed, as that functionality is now in the DB: Phase out HistoryTrackerInterace #792

[albeanth]

@john-science , @ntouran Giving this a bump.

Can we remove db/database.py? It only contains the Database class and seems to be only inherited by database3.py::Database3:

• The DataBase class doesn't have a special constructor and each of the abstract methods are overwritten by Database3. Seems like it can be safely removed.

[john-science]

Agreed. I'll just rip it out.

Also, it has always bugged me that we have to keep calling "Database3" as the default database name. I'm sure there is some history there, but it doesn't make a lot of sense now.

[albeanth]

Agreed. I'll just rip it out.

Score!

[john-science]

PR is up: #873