-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Performance #36
Comments
Also the file |
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. |
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 As an example at the beginning the API would call The negative to this approach however is:
|
I haven't thought about it too much, but my initial reaction is that we would create more work than necessary by trying that. |
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 |
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 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. |
@jstac @spencerlyon2 Great - Good points - I agree. This puts clear delineation between |
@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 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. |
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. |
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. |
@spencerlyon2 Can this issue now be closed? I have on my to do list to get vbench or some performance tracker up and running. |
Yep, if it is on your radar we don't need the issue |
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.
The text was updated successfully, but these errors were encountered: