Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Making HARK both Python 2.7 and 3.6 compatible
============================================== This commit finalizes the work of making HARK Python 2 and Python 3 compatible. The following summary list below enumerates the files have all been updated through the following process. 1. Run futurize and examine the output. If "old_div" is suggested, fix manually and re-run futurize (see detailed discussion at end). 2. Use a diff tool (Meld) to manually compare the resulting file to previous work done in the py2py3compatibility branch here: https://github.com/npalmer-professional/HARK-1/tree/py2py3compatibility 3. If it exists, replace "if __name__ == '__main__':" with "def main():" so file executable code is easy to run in the new package structure. 4. Compare output, I: Run the new ("futurized") code in both Python 3.6 and Python 2.7 and compare results to ensure they are the same. 5. Compare output, II: Run the new ("futurized") code in Python 3.6 compare it to the current econ-ARK/HARK master (econ-ark@a85ff0a) run in Python 2.7 and confirm that outputs match. All of these steps have been done for the files outlined in the following summary list. The final section contains detailed notes on this process and the list below. (I) Summary List of Updated Code Files ====================================== For bullets 1-6 below, each of the files was successfully run through steps 1-5 above. For bullets 7 and 8, cstwMPC and cAndCwithStickyE, the main files could be run but axuiliary files (eg. for making plots, or for running without a Markov structure) could not be run, and I could not solve the problems in a straighforward way. Those two should be examined by the authors of those models. (In addition, a few code files that were explicitly marked as "old" were not run through the update steps outlined above at all.) Finally, the "testing" files in bullet 9 were updated and all of them ran, but it is unclear from their output if they accomplish what their original author intended. 1. [x] ./HARK/ - [x] estimation.py - [x] interpolation.py - [x] core.py - Note: in the method HARK.core.Market.solveAgents(), I added a try/catch statement to execute in serial using "multiThreadCommandsFake" if the multi-processing version "multiThreadCommands" failed for any reason, with a printed warning. This was needed because of an unexpected multi-processing error under Python 3 encountered in FashionVictimModel. This should be fixed but not in this commit. See more under FashionVictimModel below. - [x] parallel.py - [x] simulation.py - [x] utilities.py 2. [x] ./HARK/ConsumptionSaving/ - [x] ConsIndShockModel.py - [x] ConsAggShockModel.py - [x] Ran the two fastest-running examples for testing. - [x] TractableBufferStockModel.py - [x] ConsRepAgentModel.py - [x] ConsGenIncProcessModel.py - [x] RepAgentModel.py - [x] ConsPrefShockModel.py - [x] ConsumerParameters.py - [x] ConsMedModel.py - Note: this code throws up a number of warnings at the start, under both Py2 and Py3. The author should confirm these are fine. - [x] ConsMarkovModel.py - Note: there is a missing attribute for one of the tests. However every other past of the file runs, and external conversation indicates this is not a crucial component that is causing the error. Thus I am still marking this as "checked." - The error encountered: - "AttributeError: 'MarkovSmallOpenEconomy' object has no attribute 'PermShkAggDstn'" - TODO: create a formal issue for this. 3. [x] ./HARK/ConsumptionSaving/Demos/ - [x] Fagereng_demo.py - [x] MPC_credit_vs_MPC_income.py - [x] NonDurables_During_Great_Recession.py - [x] Chinese_Growth.py 4. [x] ./HARK/ConsumptionSaving/ConsIndShockModelDemos/ - [x] TryAlternativeParameterValues.py - Note: renamed Try-Alternative-Parameter-Values.py to TryAlternativeParameterValues.py to make it work with the package structure. Could not import it with dashes in the name. 5. [x] ./HARK/FashionVictim/ - [x] FashionVictimParams.py - [x] FashionVictimModel.py - NOTE: Under Python 3, we now get this error when running FashionVictim.main(): - AttributeError: Can't pickle local object 'FashionVictimType.update.<locals>.<lambda>' - This appears to be a difference in how pickle works under Py3 vs Py2. This should be examined further. - TODO: create formal issue. 6. [x] ./HARK/SolvingMicroDSOPs/ - [x] SetupSCFdata.py - [x] EstimationParameters.py - [x] StructEstimation.py - Note: this originally broke with the error: - AttributeError: 'TempConsumerType' object has no attribute 'PermGroFacAgg' - This was easily fixed by adding that to the Params dictionary. A more satisfying fix may be done in the future. 7. [ ] ./HARK/cstwMPC/ - [x] cstwMPC.py - [x] SetupParamsCSTW.py - [?] MakeCSTWfigs.py - This file could not be run; missing code could not be found. Error encountered: - "NameError: name 'InfiniteType' is not defined" - TODO: create a formal issue for this. - [?] MakeCSTWfigsForSlides.py - This file could not be run; missing files could not be found. Error encountered: - FileNotFoundError: [Errno 2] No such file or directory: './HARK-1/HARK/cstwMPC/Results/LCbetaPointNetWorthLorenzFig.txt' - TODO: create a formal issue for this. - [ ] cstwMPCold.py - not updated because marked as "old" - [ ] SetupParamsCSTWold.py - not updated because marked as "old" 8. [ ] ./HARK/cAndCwithStickyE/ - [x] StickyEmodel.py - [x] StickyE_MAIN.py - [x] StickyEtools.py - [x] StickyEparams.py - [:] StickyE_NO_MARKOV.py - Note: this file has been updated, however final could not be completed because this file crashes my computer every time I run it. Specifically, when it is run it fills memory immediately and the computer crashes after the first plot is display. I have not found a way around it. - TODO: create a formal issue for this. 9. [x] ./HARK/Testing/ - Note: these will all run but unclear if doing what originally intended. - [x] MultithreadDemo.py - [x] ModelTesting.py - [x] HARKutilities_UnitTests.py - [x] Comparison_UnitTests.py - [x] TractableBufferStockModel_UnitTests.py (II) Detailed Discussion of the Python 2.7 + Python 3.6 Compatability Update ============================================================================= Here are some more detailed notes on the merge: (II.a) Futurize and "old_div" ----------------------------- The "futurize" function was extremely useful for identifying a number changes that would make code comaptible under both Py2 and Py3, however it did have one quirk: it often tried to import a function called "old_div" from a package called "past.utils," and replace all instances of "x/y" in a file with "old_div(x,y)." The "old_div" function replicates the old Py2 implicit integer division -- that is, 1/3 in Py2 evaluates to 0. This has been a long-standing stumbling-block for scientific computing, as one always needs to ensure that at least one of "x" or "y" in "x/y" is a float if one wants float division. This was changed in Python 3: 1/3 = 0.333... in Py3, as one would typically expect. The old Py2 "/" behavior can introduce hard-to-find bugs in scietific code if this is not remembered, thus "old_div" is not desireable on these grounds alone. In addition, the package that contains "old_div" did not come with my default installation of Anaconda Python. Thus if we were to use "old_div" we would need to force all users to install an extra package that makes their code more likely to have errors if they change something deep in a file somewhere, and don't notice that they accidentally made some value an int when it used to be a float thus introducing a bug. Now, contra the two points above, from external discussion I learned that code authors *have* used the implicit integer division from python 2.7 in a few select places because they needed integer divsion. Thus my updating process for all files listed in section (I) above was the following: 1. If futurize indicated that a file needed to have "old_div" in it, I saved the futurize output (which displays all proposed changes) to this file: https://github.com/npalmer-professional/HARK-1/wiki/files_that_may_need_integer_division (see the raw text file for easier reading, as github's markdown translates some of the futurize output syntax as markdown syntax). 2. I then manually inserted "from __future__ import division" into the file under question and re-ran futurize; typically this vastly shortened the futurize output. 3. Due to the vast amount of output pruduced by futurize with "old_div," I adopted a manual search procedure to more efficiently identify when integer division was truely needed. This is outlined in https://github.com/npalmer-professional/HARK-1/wiki/files_that_may_need_integer_division 4. When I found legitimate need for integer division, I used the explicit Python operator "//" which imposes integer division under both Python 2 and Python 3. 5. Finally step 5, "Compare output, II", in my overall testing procedure outlined above, should catch any instance where a diversion in division behavior has dramatic effects. None were found. The following 5 files appear to legitimately need integer division; I outline exactly where in https://github.com/npalmer-professional/HARK-1/wiki/files_that_may_need_integer_division : - ./HARK/cstwMPC/cstwMPC.py - ./HARK/cAndCwithStickyE/StickyE_NO_MARKOV.py - ./HARK/cAndCwithStickyE/StickyE_MAIN.py - ./HARK/cAndCwithStickyE/StickyEtools.py - ./HARK/cAndCwithStickyE/StickyEparams.py (II.b) Merging with py2py3compatibility --------------------------------------- A similar process to the one described above was done for the files in the py2py3compatibility branch here: https://github.com/npalmer-professional/HARK-1/tree/py2py3compatibility Merging these changes with the current version of HARK that has undergone the "installability" update proved to be tricky. It turns out that Python 2 and Python 3 handle imports inside a package structure a little differently -- Python 2 allowed some "fuzzy" importing while Python 3 did not. Package structures also changes the file order, so a simple diff had to be run with a little more manual effort. In addition, a "whitespace cleanup" that we previosly did on the HARK code meant that the work done under py2py3compatibility was not easy to spot in the diff -- often entire files showed up has having most lines changed. This required a closer examiniation of each file's diff. (II.c) Relacing "if __name__ == '__main__'" with "def main()" -------------------------------------------------------------- Executable code under an "if __name__ == '__main__'" section in a file cannot be easily executed under the package import system. After some discussion with Jason about this, I replaced all instances of "if __name__ == '__main__'" with "def main()," so now there is simply a "main" function associated with the file. So for example, the following code from the Python command line: from HARK.ConsumptionSaving import ConsIndShockModel ConsIndShockModel.main() will now execute the old "if __name__ == '__main__'" code from ConsIndShockModel. (II.d) Explicit Path Naming to Read/Write to Disk inside a Package ------------------------------------------------------------------- In a number of files inside HARK/cstwMPC/ and HARK/cAndCwithStickyE/, there were a number of calls to read files from, and write files to, disk. This works differently inside a package structure because of how the path is used by Python there. I fixed these errors by addin these two lines: import os my_file_path = os.path.dirname(os.path.abspath(__file__)) ...which provided the absolute path to the file currently executing. This works both inside and outside packages. Then in every instance where a read from disk or a write to disk was needed and referred to the current folder, i.e. had "./" in the pathname to be read/written, I instead used the "my_file_path" to get the correct location. (II.e) Additional Notes for cAndCwithStickyE -------------------------------------------- Note 1: In cAndCwithStickyE.StickyEtools, I had to make all strings "raw" strings because otherwise the "\u" characters messed things up in Python 3 because the are interpreted as unicode literals (a new Python 3 feature). Essentially that means starting all strings with the letter 'r'. For example: "a string" becomes r"a string" Without the change the cAndCwithStickyE.StickyEtools crashes when run under Python 3. This needs to be confirmed to produce reasonable output by someone who uses cAndCwithStickyE. Note 2: In cAndCwithStickyE.StickyE_MAIN, the only procedures that were tested were those activated by these flags, because otherwise it took too long: # Choose which models to do work for do_SOE = False do_DSGE = False do_RA = True # Choose what kind of work to do for each model run_models = True # Whether to solve models and generate new simulated data calc_micro_stats = False # Whether to calculate microeconomic statistics (only matters when run_models is True) make_tables = False # Whether to make LaTeX tables in the /Tables folder use_stata = False # Whether to use Stata to run the simulated time series regressions save_data = False # Whether to save data for use in Stata (as a tab-delimited text file) run_ucost_vs_pi = False # Whether to run an exercise that finds the cost of stickiness as it varies with update probability run_value_vs_aggvar = False # Whether to run an exercise to find value at birth vs variance of aggregate permanent shocks Further testing by another person familiar with the code should likely be done.
- Loading branch information
1 parent
73eff9a
commit ebe301f