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.

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

Endogenize investments #80

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

Endogenize investments #80

wants to merge 11 commits into from

Conversation

hmgaudecker
Copy link
Member

@hmgaudecker hmgaudecker commented Sep 26, 2024

Endogeneity of investments means that concurrent states influence the choice of investments. Skillmodels currently has no way of including that.

In this PR, we achieve it this by modelling this through transition functions. When investment factors are present, each period is split into two sub-periods. The first sub-period is for states, which can influence investments in the second sub-period.

  • Update model processing, allowing for an entry is_investment for factors. Add periods_raw (in the original data) in addition to periods (the doubled-up version used internally by skillmodels)
  • Stack data, appropriately including measurements in sub-periods
  • Add constraints so that factors are just carried along through sub-periods where state/investment is not relevant

Limitations:

  • There can be no overlap between measurements for states and for investments because to skillmodels, they live in disjunct periods
  • Anchoring is not supported when investment factors are present
  • Stages are not supported when investment factors are present (would require extra logic for the constraints, might e doable without much work)

Copy link

codecov bot commented Sep 26, 2024

Codecov Report

Attention: Patch coverage is 96.31336% with 8 lines in your changes missing coverage. Please review.

Project coverage is 91.67%. Comparing base (b7975a6) to head (50ce8d0).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/skillmodels/process_model.py 92.59% 4 Missing ⚠️
src/skillmodels/check_model.py 88.88% 2 Missing ⚠️
src/skillmodels/process_data.py 96.00% 1 Missing ⚠️
src/skillmodels/simulate_data.py 80.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #80      +/-   ##
==========================================
+ Coverage   91.43%   91.67%   +0.24%     
==========================================
  Files          40       40              
  Lines        3070     3244     +174     
==========================================
+ Hits         2807     2974     +167     
- Misses        263      270       +7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@janosg janosg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are endogenous factors not compatible with anchoring? And why not with stages? Especially stages are so orthogonal to everything else (they are just fixed constraints) that I don't think it would be hard.

For example, if you had chosen to implement all the logic for endogenous factors by transforming the user written model dict, the new stages could have been:

new_stagemap = list(np.repeat(old_stagemap, 2))

anchoring (dict): Dictionary with information about anchoring.
See :ref:`anchoring`
has_investments (bool): Whether the model has any investment factors
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there is a big gain from calling everything that is measured with error a latent factor. That is why in skillmodels, investment factors always have been just regular factors. I would call them endogenous factors now, but I would not use the term investments (and definitely not the abbreviation inv) to distinguish endogenous from exogenous factors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants