-
Notifications
You must be signed in to change notification settings - Fork 10
Add L0 Regularization, make a better small ECPS #364
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
Conversation
…zation-in-enhanced_cps.py Implement HardConcrete L0 regularization
…de-block-for-diagnostics Deduplicate reweighting diagnostics
juaristi22
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! Excited to see what this dataset can do!
…nts-to-logging Replace prints with logging
…data into bogorek-l0
|
Thanks!! But a bit paranoid of course about this new dataset. Can you add to this PR:
All of the above compared to the regular ECPS before this PR. @PavelMakarchuk can you share 3 policy jsons/structural reforms that @baogorek can run, along with expected revenue numbers. |
|
@nikhilwoodruff I copied over all the tests that the ecps has to pass, and it passed all but one. The one it failed on was; So I suppose this sparse solution happened not to include anyone with NONE for ssn_card_type. How big of a deal do you think that is? FYI, I made the threshold in the test very large so it would pass and log it out, so eventually I need to fix that. The calibration log looks pretty good! |
|
Given the extent to which OBBBA affects the estimated 13 million undocumented immigrants, I think it's problematic to report that zero exist. |
|
OK @baogorek could you increase the number til we don't hit this error? |
|
How many records in this dataset? |
|
@nikhilwoodruff , @MaxGhenis I'm passing all tests, including the ssn_card_type test with the original threshold (see starting at line 150 of policyengine_us_data/tests/test_datasets/test_sparse_enhanced_cps.py). There are 5,971 non-zero weights in this run, as can be seen at the end of the build data sets log. There is some non-determinism and I've seen the loss go lower locally, so I could investigate that, or just double the number of epochs for the final run. |
MaxGhenis
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to see that epochs and other hyperparameters solved it
|
@nikhilwoodruff this is passing all of the tests that the ordinary ecps is passing (the previous failure was due to not storing enums correctly in the h5). Is this enough to go through with this as a sparse MVP? If I need to test with OBBBA reforms, @PavelMakarchuk , I will need your help. There are slightly over 5k households in this sparse dataset, which is enough to estimate the ssn card type == "NONE" total within the original accuracy thresholds. I believe this model is solid. |
|
OK LGTM. We have dataset versioning anyway. Let's merge |
* initial commit of L0 branch * Add HardConcrete L0 regularization * l0 example completed * removing commented code * pre lint cleanup * post-lint cleanup * Refactor reweighting diagnostics * removed _clean from names in the reweighting function * modifying print function and test * Convert diagnostics prints to logging * removing unused variable * setting high tolerance for ssn test just to pass * linting * fixed data set creation logic. Modified parameters * docs. more epochs
* Use normal runner in PR tests * added the 3.11.12 pin * cps.py * adding diagnostics * lint * taking out bad targets * fixing workflow arg passthrough * deps and defaults * wrong pipeline for manual test * trying again to get the manual test to work * reverting to older workflow code * cleaning up enhanced_cps.py * Update package version * removing github download option. Switching to hugging face downloads * changelog entry * reverting the old code changes workflow * Update package version * start cleaning calibration targets * add us package to dependencies * update csv paths in tests too * manual test * pr * updates * trying to get the right workflow to run * taking out the token * ready for review * Update package version * adding diagnostics * taking out bad targets * fixing workflow arg passthrough * wrong pipeline for manual test * Update package version * removing github download option. Switching to hugging face downloads * reverting the old code changes workflow * remove districting file * remove duplications from merge with main * add changelog_entry * Add L0 Regularization, make a better small ECPS (#364) * initial commit of L0 branch * Add HardConcrete L0 regularization * l0 example completed * removing commented code * pre lint cleanup * post-lint cleanup * Refactor reweighting diagnostics * removed _clean from names in the reweighting function * modifying print function and test * Convert diagnostics prints to logging * removing unused variable * setting high tolerance for ssn test just to pass * linting * fixed data set creation logic. Modified parameters * docs. more epochs * Update package version * Pin microdf * adding diagnostics * taking out bad targets * Update package version * start cleaning calibration targets * trying to get the right workflow to run * ready for review * taking out bad targets * restore changes lost when merging with main * more cleanup * even more cleanup * fix file paths in new sparse ecps test * lint * fixing merge --------- Co-authored-by: Nikhil Woodruff <35577657+nikhilwoodruff@users.noreply.github.com> Co-authored-by: baogorek <baogorek@gmail.com> Co-authored-by: MaxGhenis <MaxGhenis@users.noreply.github.com> Co-authored-by: baogorek <baogorek@users.noreply.github.com> Co-authored-by: nikhilwoodruff <nikhilwoodruff@users.noreply.github.com>
Closes #197 , Closes #356 refences L1 but I think this is in the spirit of it)