QuantEcon / QuantEcon.py

A community based Python library for quantitative economics
https://quantecon.org/quantecon-py/
MIT License
1.97k stars 2.25k forks source link

Performance #36

Closed sglyon closed 9 years ago

sglyon commented 10 years ago

As part of the python 3 compatibility push I have been running through all the solutions notebooks. I noticed that the following notebooks took an extra long time to run:

This issue is merely a placeholder for us to document where we have noticed opportunities for performance improvements.

sglyon commented 10 years ago

Also the file examples/odu_vfi_plots.py.py takes a very long time to run (a few minutes).

jstac commented 10 years ago

Thanks for the comments. Both of these modules compare fast and slow algorithms, so it's OK that the slow ones are slow. We could possibly to speedups, but they would have to be done in parallel on both algorithms so that the comparisons are still valid.

sanguineturtle commented 10 years ago

I've been thinking recently about the interesting tension between the toolkit vs the teaching tool nature of the QuantEcon package. (toolkit = optimised and fast routines, teaching tool = clear and logical routines). Now that the testing infrastructure is up. It would be possible to move the current "flat" package to a quantecon/teaching sub-package and promote them to the top level namespace via the api. Then as tools and models are required a toolkit versions could be created in a subpackage (models / tools etc) or at the base level package as they current are now and alter the api reference to call the optimised version. We would then tie the two together with a test to ensure they both agree on the same results to ensure changes are propagated so that either can be used.

As an example at the beginning the API would call quantecon/teaching/asset_pricing.py unless it's optimised etc. in which case it would be replaced by /quantecon/asset_pricing.py or /quantecon/models/asset_pricing.py and a test quantecon/tests/test_compare_asset_pricing.py to compare both routines final output to ensure any changes are propagated and they provide the same answer.

The negative to this approach however is:

  1. Maintain two sets of functions and classes (but not all methods would need to be optimised)
  2. Additional tests needed to enforce common output from both implementations.
sglyon commented 10 years ago

I haven't thought about it too much, but my initial reaction is that we would create more work than necessary by trying that.

sanguineturtle commented 10 years ago

Yeah - you might be right @spencerlyon2. I'm not sure how much of the code base would actually need "replicating". An alternative would be to leave everything as is and write a quantecon/optimised subpackage and update the api to use the optimised version instead whenever they are written.

jstac commented 10 years ago

These are valuable thoughts, and clearly we're all getting our heads round this tension between (a) great toolkit and (b) simple code suitable for posting on quant-econ.net and teaching from. What I think is this:

Let's focus on turning the QuantEcon package into a fantastic toolkit, period. Everything should be coded to the best standard and run as fast as possible. It shouldn't contain any replication. It shouldn't make compromises to accommodate the lectures.

If this interferes with the lectures on quant-econ.net then I'll change the lectures. One option for me is to write a simple toy version of an algorithm and put it in the examples folder of the public repo, outside the QuantEcon package. (At the same time, the lecture would refer the reader to the 'proper' version in QuantEcon.) Another option is to add more explanation to the lectures.

I'll be handling most of the PRs so I'll be in a good position to judge when a change to the QuantEcon package requires me to change a lecture.

This can evolve over time and I don't think it requires any structural changes to the QuantEcon package at this time (apart from putting stuff like career.py into a 'models' subpackage, as already discussed). Feel free to tell me if you disagree.

sanguineturtle commented 10 years ago

@jstac @spencerlyon2 Great - Good points - I agree. This puts clear delineation between /quantecon as a toolkit and any required lecture / website support that may pop up through/examples. @jstac if/when a simpler version is written in support of the website, then we can add a test to examples/tests that will compare the simpler version with the package function.

cc7768 commented 10 years ago

@jstac @spencerlyon2 @sanguineturtle This is an important discussion to have. There are several functions that I have noticed could be vectorized (or numba-ized) to be sped up, but provide a lot of intuitive value as they are.

One example that @spencerlyon2 and I talked about was the tauchen.py file (although, on a side note, I think the file name should be approx_markov.py with a function name of tauchen). For the learning portion of quant-econ, I think the way that it is written up is excellent. It is explicit and follows the defined algorithm exactly. I wrote up the Tauchen method for an assignment this year and it was all vectorized, which provided big speed ups. It would also be a good candidate for possibly being "numba-ized."

I think putting toy versions of the algorithms within the examples folder is a good idea. I think we should hurry and get the tests branch pushed and then we can rewrite tests as we update/optimize code. I liked that the pull request for the ivp solver came with tests, that should be a requirement for a good pull request.

jstac commented 10 years ago

Absolutely. All pull requests from new or outside developers should come with tests. That will be part of the protocol for working on QuantEcon, and the protocol will be fully documented on the Wiki.

jstac commented 10 years ago

Regarding the comment on the tauchen code, I guess the beauty of numba is that in many cases the algorithm / implementation would hardly have to change.

mmcky commented 9 years ago

@spencerlyon2 Can this issue now be closed? I have on my to do list to get vbench or some performance tracker up and running.

sglyon commented 9 years ago

Yep, if it is on your radar we don't need the issue