search

PSLmodels / OG-Core

An overlapping generations model framework for evaluating fiscal policies.
https://pslmodels.github.io/OG-Core/
Creative Commons Zero v1.0 Universal
65 stars 111 forks source link

Update demographics.py to use new UN API format and token #934

Closed SeaCelo closed 3 months ago

SeaCelo commented 3 months ago

Making it compatible with un API token use. Also to fix an error when downloading the full dataset (end year = 2100) in line 377 demographics.py. This will likely create other problems, so it needs to be evaluated. Still running into an assertion error:

Demographics data: Initial Data year =  2020 , Final Data year =  2100
Traceback (most recent call last):
  File "/Users/mlafleur/Projects/OG-ZAF/tests/test_demog_jdebacker.py", line 5, in <module>
    c = Calibration(p, estimate_pop=True)
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/mlafleur/Projects/OG-ZAF/ogzaf/calibrate.py", line 98, in __init__
    self.demographic_params = demographics.get_pop_objs(
                              ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/mlafleur/opt/anaconda3/envs/ogzaf-dev/lib/python3.11/site-packages/ogcore/demographics.py", line 909, in get_pop_objs
    assert np.allclose(pop_counter_2D, pop_2D)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError
SeaCelo commented 3 months ago

cc: @jdebacker

jdebacker commented 3 months ago

@SeaCelo How would you feel about using a prompt that requests the API key (and user pastes it in) as opposed to looking for a file?

I was thinking about the former which might work more easily on GH Actions (we can save an API key to the GH secrets) and would not require users to save their API key in a specific place.

SeaCelo commented 3 months ago

@jdebacker I'm not sure what the implications are. I think we should avoid having to enter an API key every time someone uses the model. Maybe once the user pastes their token it can be saved locally as a secret? I imagine this is a quite common issue so there must be a "best practice", I just don't know it.

jdebacker commented 3 months ago

Closing in favor of #936