nickdeveaux
commented
6 years ago wow! great stuff. I don't see any major clashes with any of the pending pull requests, like #60 . Not sure if I'll be able to do a full code review.
Closed asistradition closed 6 years ago
nickdeveaux
commented
6 years ago wow! great stuff. I don't see any major clashes with any of the pending pull requests, like #60 . Not sure if I'll be able to do a full code review.
AaronWatters
commented
6 years ago Hi folks. I'm just checking back in to this repo after a long absence.
For what it's worth I would prefer pull requests to come in more bite sized chunks so it is easier to assess the impact. It seems like this pull request changes a lot of things at once that aren't necessarily directly related.
My 2p.
Aight this is the rewritten code that we were discussing today. It should be mostly a drop in replacement for the older code. There are a ton of changes, but they're all pretty straightforward. The regression engine is faster but functionally results in the same output. The MI engine is entirely python with standard dependencies, and returns mostly the same output (to float tolerance). The exception is the CLR change that was discussed today.
The workflow is lighter in memory usage; after it finishes setup, workflow.design and workflow.response should be the only heavy memory objects remaining (the rest should dereferenced for garbage collection). To mitigate problems with memory spiking during setup, workflow.async_start can be set True, and the code in startup_run will run on workflow.async_chunk number of processes. Once one process finishes startup_run, another will start, until everyone's finished. It'll then run startup_final with all the processes and move on to running the regression. MI and the regression driver do copy workflow.design and workflow.response, but not at the same time, and those copies are dereferenced for garbage collection. Peak memory usage during regression should be a little over 4 times the expression data once it's in an array, plus overhead from the interpreter. I believe that all KVS objects are torn down immediately after all of the client processes have accessed them.
The utils package has a Debug class that's just used in place of print and can be set to varying levels of verbosity. It also has a kvs_async class that has process control methods to manage staging through KVS.
The entire thing runs parallelized with python 2.7. It runs nicely in 3.6 as well, provided that there is only one process. There is some desync bug in 3.6 that turns the output to garbage with multiple processes (it fails silently, so if you run this on the HPC with 3.6 it'll just give you nonsense). It passes all of the existing tests, but I haven't implemented any new ones and it may be worth comparing your specific output before & after this branch.