Skip to content
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.

Sign up for GitHub

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

Jump to bottom

API for parameters does not take np.array type #1198

Closed
alanlujan91 opened this issue Jan 2, 2023 · 14 comments
Closed

API for parameters does not take np.array type #1198

alanlujan91 opened this issue Jan 2, 2023 · 14 comments
Milestone
1.0.0

Comments

@alanlujan91
Copy link
Member

alanlujan91 commented Jan 2, 2023 •
edited

Describe the bug

Logging results in errors on DistributionOfWealthMPC, see econ-ark/DistributionOfWealthMPC#4

HARK/HARK/ConsumptionSaving/ConsIndShockModel.py

Lines 1948 to 1950 in 65c76c8

_log.info(messages[self.conditions[name]].format(self))
if verbose_messages:
_log.debug(verbose_messages[self.conditions[name]].format(self))

To Reproduce
Steps to reproduce the behavior:

  1. Go to [WIP] Updates to code  DistributionOfWealthMPC#4
  2. Install PR environment with HARK@master
  3. Run cstwMPC.ipynb
  4. See error
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
File c:\Users\alujan\miniconda3\envs\cstwMPC\lib\site-packages\HARK\core.py:1299, in Market.solve_agents(self)
   1298 try:
-> 1299     multi_thread_commands(self.agents, ["solve()"])
   1300 except Exception as err:

File c:\Users\alujan\miniconda3\envs\cstwMPC\lib\site-packages\HARK\parallel.py:51, in multi_thread_commands(agent_list, command_list, num_jobs)
     50 if len(agent_list) == 1:
---> 51     multi_thread_commands_fake(agent_list, command_list)
     52     return None

File c:\Users\alujan\miniconda3\envs\cstwMPC\lib\site-packages\HARK\parallel.py:31, in multi_thread_commands_fake(agent_list, command_list, num_jobs)
     29 for command in command_list:
     30     # TODO: Code should be updated to pass in the method name instead of method()
---> 31     getattr(agent, command[:-2])()

File c:\Users\alujan\miniconda3\envs\cstwMPC\lib\site-packages\HARK\core.py:412, in AgentType.solve(self, verbose)
    409 with np.errstate(
    410     divide="ignore", over="ignore", under="ignore", invalid="ignore"
    411 ):
--> 412     self.pre_solve()  # Do pre-solution stuff
    413     self.solution = solve_agent(
    414         self, verbose
    415     )  # Solve the model by backward induction

File c:\Users\alujan\miniconda3\envs\cstwMPC\lib\site-packages\HARK\ConsumptionSaving\ConsIndShockModel.py:3115, in IndShockConsumerType.pre_solve(self)
   3114 if not self.quiet:
-> 3115     self.check_conditions(verbose=self.verbose)

File c:\Users\alujan\miniconda3\envs\cstwMPC\lib\site-packages\HARK\ConsumptionSaving\ConsIndShockModel.py:3292, in IndShockConsumerType.check_conditions(self, verbose)
   3291 #        self.check_GICRaw(verbose)
-> 3292 self.check_GICNrm(verbose)
   3293 self.check_GICAggLivPrb(verbose)

File c:\Users\alujan\miniconda3\envs\cstwMPC\lib\site-packages\HARK\ConsumptionSaving\ConsIndShockModel.py:3140, in IndShockConsumerType.check_GICNrm(self, verbose)
   3139 verbose = self.verbose if verbose is None else verbose
-> 3140 self.check_condition(name, test, messages, verbose, verbose_messages)

File c:\Users\alujan\miniconda3\envs\cstwMPC\lib\site-packages\HARK\ConsumptionSaving\ConsIndShockModel.py:1948, in PerfForesightConsumerType.check_condition(self, name, test, messages, verbose, verbose_messages)
   1947 set_verbosity_level((4 - verbose) * 10)
-> 1948 _log.info(messages[self.conditions[name]].format(self))
   1949 if verbose_messages:

TypeError: unhashable type: 'numpy.ndarray'

During handling of the above exception, another exception occurred:

TypeError                                 Traceback (most recent call last)
Cell In[4], line 13
      9 do_liquid = False            # Matches liquid assets data when True, net worth data when False
     12 os.chdir(path_to_models)
---> 13 exec(open('cstwMPC_MAIN.py').read())

File <string>:970

File c:\Users\alujan\miniconda3\envs\cstwMPC\lib\site-packages\scipy\optimize\_zeros_py.py:783, in brentq(f, a, b, args, xtol, rtol, maxiter, full_output, disp)
    781 if rtol < _rtol:
    782     raise ValueError("rtol too small (%g < %g)" % (rtol, _rtol))
--> 783 r = _zeros._brentq(f, a, b, xtol, rtol, maxiter, args, full_output, disp)
    784 return results_c(full_output, r)

File <string>:961, in <lambda>(center)

File <string>:714, in get_KY_ratio_difference(economy, param_name, param_count, center, spread, dist_type)

File <string>:194, in solve(self)

File c:\Users\alujan\miniconda3\envs\cstwMPC\lib\site-packages\HARK\core.py:1313, in Market.solve_agents(self)
   1303     self.print_parallel_error_once = False
   1304     print(
   1305         "**** WARNING: could not execute multi_thread_commands in HARK.core.Market.solve_agents() ",
   1306         "so using the serial version instead. This will likely be slower. "
   (...)
   1311         err,
   1312     )  # sys.exc_info()[0])
-> 1313 multi_thread_commands_fake(self.agents, ["solve()"])

File c:\Users\alujan\miniconda3\envs\cstwMPC\lib\site-packages\HARK\parallel.py:31, in multi_thread_commands_fake(agent_list, command_list, num_jobs)
     28 for agent in agent_list:
     29     for command in command_list:
     30         # TODO: Code should be updated to pass in the method name instead of method()
---> 31         getattr(agent, command[:-2])()

File c:\Users\alujan\miniconda3\envs\cstwMPC\lib\site-packages\HARK\core.py:412, in AgentType.solve(self, verbose)
    406 # Ignore floating point "errors". Numpy calls it "errors", but really it's excep-
    407 # tions with well-defined answers such as 1.0/0.0 that is np.inf, -1.0/0.0 that is
    408 # -np.inf, np.inf/np.inf is np.nan and so on.
    409 with np.errstate(
    410     divide="ignore", over="ignore", under="ignore", invalid="ignore"
    411 ):
--> 412     self.pre_solve()  # Do pre-solution stuff
    413     self.solution = solve_agent(
    414         self, verbose
    415     )  # Solve the model by backward induction
...
-> 1948 _log.info(messages[self.conditions[name]].format(self))
   1949 if verbose_messages:
   1950     _log.debug(verbose_messages[self.conditions[name]].format(self))

TypeError: unhashable type: 'numpy.ndarray'

Expected behavior
DistributionOfWealthMPC should work with HARK@master branch. It works when these lines are commented out.

Screenshots
If applicable, add screenshots to help explain your problem.

Desktop (please complete the following information):

  • OS: Windows
  • Browser chrome
  • Econ-ARK and Python version: dev branch HARK@master, python 3.9.15, numpy 1.23.5 pypi_0 pypi

Additional context
@llorracc @wdu9 @dedwar65 critical to update DistributionOfWealthMPC
@alanlujan91
Copy link
Member Author

@sbenthall could you help me with this? I'm not familiar with the logging features of HARK

@sbenthall sbenthall added this to the 0.13.0 milestone Jan 4, 2023
@sbenthall
Copy link
Contributor

I'm confused about how to reproduce this error.

  1. Why is this issue on HARK and not in the DistributionOfWealthMPC repository? Is that a mistake?
  2. I'm trying to reproduce the error, but running cstwMPC.py gets me this error:
$ python cstwMPC_MAIN.py 
Traceback (most recent call last):
  File "/home/sb/projects/econ-ark/DistributionOfWealthMPC/Code/cstwMPC_MAIN.py", line 65, in <module>
    if param_name == "DiscFac":
NameError: name 'param_name' is not defined
  1. Going into the Code/ directory and trying to run cstwMPC_MAIN.py get me:
$ python cstwMPC_MAIN.py 
Traceback (most recent call last):
  File "/home/sb/projects/econ-ark/DistributionOfWealthMPC/Code/cstwMPC_MAIN.py", line 65, in <module>
    if param_name == "DiscFac":
NameError: name 'param_name' is not defined

So I haven't gotten to the logging issue yet.

@sbenthall
Copy link
Contributor

OK, I've replicated this error by working from the cstwMPC.ipynb notebook.

@alanlujan91
Copy link
Member Author

Correct, this happens in the cstwMPC.ipynb file. Probably should have put it on DistributionOfWealthMPC but I assumed fixing it would require changes in HARK.

@sbenthall
Copy link
Contributor

The error is happening because:

  • The value of self.conditions['GICRaw'] is an np.array, [True].
  • This value is being passed to messages[self.conditions[name]] as a key to the dictionary messages
  • The keys of messages are expected to be booleans, not arrays of booleans.
  • The condition that is causing the failure is the one tested by check_GICNrm. This is hard to discover, because there is an error in the code whereby this conditions has the name GICRaw -- probably an error from copying and pasting code.
  • The reason why the result of its test is an array [True] is because self.thorn is a single-valued array.
  • The reason why self.thorn is a single-valued array is because self.DiscFac is a single-valued array.

Did self.DiscFac become array-valued in some recent change to HARK? Is that because it is time-varying? Or otherwise multivalued?

If so, how shall we compute thorn?

@dedwar65
Copy link
Contributor

dedwar65 commented Jan 10, 2023 via email

@sbenthall
Copy link
Contributor

@dedwar65 The notebook is here:

https://github.com/econ-ark/DistributionOfWealthMPC/blob/master/cstwMPC.ipynb

If I recall correctly, the whole point of the DistributionOfWealthMPC paper is that the DiscFac is being estimated.
Perhaps it is the estimation code of cstwMPC that is returning an array value for the DiscFac when HARK expects a scalar value.

@alanlujan91 do you know if this logging error happens with HARK pinned to 0.12, or is it the updates to get it to work with master that are causing the break?

@sbenthall
Copy link
Contributor

  • Patch to the conditions code to take 1st element of DiscFac of single valued array
  • Complain if DiscFac has more than one value in its array.

@alanlujan91
Copy link
Member Author

@sbenthall error does not happen in econ-ark=0.12

@sbenthall
Copy link
Contributor

Thinking this over -- I am doubtful that it is a good idea to patch HARK with this change.

There is already something in check_conditions that checks to make that the model is an infinite horizon problem with only a single stage:

https://github.com/econ-ark/HARK/blob/master/HARK/ConsumptionSaving/ConsIndShockModel.py#L2054-L2055

So it would require some severe abuse of the API if DiscFac were a list instead of a scalar.

Since this was not a problem with earlier HARK versions, it is most likely that this bug was introduced in DistributionofWealthMPC.

In other words, HARK is working as expected, and the fix should be done in the DistributionofWealth repo.

@sbenthall
Copy link
Contributor

Hmmm... there is one condition under which it would make sense to patch HARK, but it would be a more general issue.

Currently, we have scalar values (e.g. floats) for parameters, and optionally lists if a parameter is time-varying.
I.e. I believe it is invalid, according to the API, to have array-values parameters (though we may be doing that in some places because both lists and arrays are 'iterable'). (Indeed, the 'must be lists' according to @mnwhite in #664 )

We could conceivably change the HARK API to accomodate array-valued parameters. Maybe that could be part of a broader effort to make the API more strongly type. This is discussed here: #664

But this is a bigger effort than should be done for the 0.13.0 release.

@sbenthall sbenthall modified the milestones: 0.13.0, 1.0.0 Jan 12, 2023
@sbenthall sbenthall removed their assignment Jan 12, 2023
@llorracc
Copy link
Collaborator

This relates to the project that @alanlujan91 and I had sketched in which we would basically make everything age-varying (in principle). We should discuss the way forward in depth at some future meeting.

@sbenthall sbenthall changed the title Logging errors API for parameters does not take np.array type Jan 13, 2023
@alanlujan91
Copy link
Member Author

I think I found the bug and it is unrelated to logging or Discount Factor being time varying. Since HARK 0.11 the representation of discrete distributions has changed, from 1 dimensional vectors to 2 dimensional where the second dimension indexes the random variable for multidimensional random variables.

@dedwar65
Copy link
Contributor

dedwar65 commented Jan 20, 2023 via email

@sbenthall sbenthall mentioned this issue Feb 16, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.0.0
Development

No branches or pull requests
4 participants
@sbenthall @llorracc @alanlujan91 @dedwar65