-
-
Notifications
You must be signed in to change notification settings - Fork 198
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
split cstwMPC into general tools and example code #449
Comments
The cstwMPC code is now in an awkward position. They have been slated for removal for almost 10 months, but blocking on @llorracc making some kinds of changes that are not otherwise ticketed up. The problem is that as this is now unsupported code with no automated tests, it may not work as the HARK library is updated. This code should be moved into a private repository and worked into a reusable HARK contribution, |
Actually, it should become a REMARK. Just haven't had time to figure out what bits might be worth importing into HARK as functions, and had a nagging feeling that some of the DemARKs or REMARKs might rely on certain things being in the cstwMPC directory structure. However, to the extent that that was true, it may have just been parameters and the data for the Survey of Consumer Finances; the former have now been reorganized, but I don't think we've designated a standard location for the SCF data (or any other data that we want to provide for general-purpose use). Probably we should do that, then change the [Dem]/[REM]ARKs that use the SCF data, then we can remove cstwMPC from main HARK. |
It’s used in "KrusselSmith" and "Structural estimates from empirical MPC”. Do we keep the data in both REMARK and DemARK?
… On 08-Apr-2020, at 10:48 PM, Christopher Llorracc Carroll ***@***.***> wrote:
Actually, it should become a REMARK. Just haven't had time to figure out what bits might be worth importing into HARK as functions, and had a nagging feeling that some of the DemARKs or REMARKs might rely on certain things being in the cstwMPC directory structure. However, to the extent that that was true, it may have just been parameters and the data for the Survey of Consumer Finances; the former have now been reorganized, but I don't think we've designated a standard location for the SCF data (or any other data that we want to provide for general-purpose use). Probably we should do that, then change the [Dem]/[REM]ARKs that use the SCF data, then we can remove cstwMPC from main HARK.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub <#449 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABI5RFHORPMH4EHIRZ5YB3DRLSWYBANCNFSM4JYSIZBQ>.
|
These data are likely to be used by future REMARKs and DemARKs and projects that people develop with the toolkit. They are kind of like the utility tools in HARK.utilities in that sense -- provide a standard benchmark. We should probably give some thought about how to store a small set of standardized datasets like this. Am sure there is lots of "best practice" out there to take advantage of. |
How about I put it as-is into a branch of the REMARK repository, then remove it from HARK. Then it can be refined into a proper REMARK and committed to REMARK master. Then later, when you get to it, you can figure out what parts of it can be contributed reusably into HARK. |
That's fine -- except that before doing that we need to figure out how any existing REMARKs/DemARKs that use stuff from it should be fixed so they won't break when we move it. |
Well, the REMARKs should be pegged to a fixed version of HARK. |
On 08-Apr-2020, at 11:44 PM, Sebastian Benthall ***@***.***> wrote:
Well, the REMARKs should be pegged to a fixed version of HARK.
What if we have future REMARKs which may use the SCF data? We need to figure out a way of keeping this data around.
We can create a datasets/ folder in HARK to store this data, this is used to many other projects to distribute data (scikit-learn comes to mind).
… I believe the current procedure for the DemARKs is to make the change and then see if the tests fail.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub <#449 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABI5RFDQIXA4GNW7LS55FA3RLS5ILANCNFSM4JYSIZBQ>.
|
Eventually, yes, a "completed" remark should be pegged. The only REMARK about which we can say for sure that it is "completed" is the Portfolio blog post for TFI -- at least, it will be completed in the sense that we should have a frozen archival version. However, that gets us back to the other issue we've been round and round about: Versioning for REMARKs and DemARKs, because the Porftolio blog post uses the CGMPortfolio REMARK. |
This is a great idea. +1 to it from me. This is what it looks like in scikit-learn: |
Sounds good to me. But not sure we want to standardize on csv -- that's a very limited data structure. SQL would be overkill. Does pandas have a standardized file structure? Maybe we should standardize on that. |
On 08-Apr-2020, at 11:59 PM, Christopher Llorracc Carroll ***@***.***> wrote:
Sounds good to me.
But not sure we want to standardize on csv -- that's a very limited data structure. SQL would be overkill. Does pandas have a standardized file structure? Maybe we should standardize on that
Let's stick with csv, its easy to distribute, has native support with pandas and no need to worry about if it work across platforms.
… —
You are receiving this because you commented.
Reply to this email directly, view it on GitHub <#449 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABI5RFHKQ2KC3FO75NM74KLRLS677ANCNFSM4JYSIZBQ>.
|
If CSV is good enough for sci-kit learn, I think it's good enough for us. |
Regarding software versions and releases... I don't know how I can make my views on this any clearer, but I'll try again. If every time there's a change to HARK, it's necessary to check every downstream use of it in some other library, that quickly makes it impossible to make any changes to HARK. I think that if there's an issue with REMARK's dependency management and contribution process, we should discuss it and fix it as part of our determination of what REMARKs are. But these processes need to be decoupled if HARK is going to go anywhere. |
Your views are clear, and I am convinced they are right. My point is basically the same as yours: We have not achieved a comparable degree of clarity or discipline about version control and management for REMARKs and DemARKs, and we need to do so. |
Ok, here is an issue for discussion the REMARK release and dependency locking process. |
It looks like cstwMPC is already a REMARK, via a submodule link, to a private llorrocc repository: https://github.com/llorracc/cstwMPC Is the version in the HARK/ library different from this REMARK version significantly? |
One possibility we haven't considered yet in this thread is for whatever DEMARKs (or REMARKs?) that depend on this get the data and code from the cstwMPC REMARK itself. |
The These are the offending lines: This means some downstream DemARKS that use this code are broken as well. I could make a PR to fix these before the 0.10.6 release, but it will take a little time. I have other obligations today. We could also formally deprecated this code for now and delay the next release of the DemARKs until this issue with cstwMPC migration is resolved. This latter option would be my preference.
|
Agreed. But fixing the DemARKs is top priority after the release. |
Ok. Fixing the DemARKs will require either:
In my view, the latter is more preferable. Either way, it is going to depend on some critical input/action from Chris. |
I agree, the latter is preferable. The chief thing that needs to be done, I think, is to define a place in HARK for the SCF dataset that these tools all use. Many toolkits have a few 'sample' datasets that are distributed with the toolkit, and for which there are easy-to-use commands to obtain the data because the toolkit "knows" where the data should be. That's the preferred endpoint. We don't need to get all the way to that endpoint right now, but it might be useful to know what the endpoint is as we decide how to do this now. |
#415 (comment)
From @llorracc
This issue is for this cstwMPC related task.
The text was updated successfully, but these errors were encountered: