[WIP] Abstract Variables/Symbols #1308

alanlujan91 · 2023-07-19T18:45:47Z

Please ensure your pull request adheres to the following guidelines:

Tests for new functionality/models or Tests to reproduce the bug-fix in code.
Updated documentation of features that add new functionality.
Update CHANGELOG.md with major/minor changes.

codecov · 2023-07-19T19:03:36Z

Codecov Report

Patch coverage has no change and project coverage change: -0.40% ⚠️

Comparison is base (37134b9) 72.55% compared to head (f41c944) 72.16%.
Report is 30 commits behind head on master.

❗ Current head f41c944 differs from pull request most recent head e4d2850. Consider uploading reports for the commit e4d2850 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1308      +/-   ##
==========================================
- Coverage   72.55%   72.16%   -0.40%     
==========================================
  Files          78       79       +1     
  Lines       13009    13080      +71     
==========================================
  Hits         9439     9439              
- Misses       3570     3641      +71

Files Changed	Coverage Δ
HARK/variables.py	`0.00% <0.00%> (ø)`

... and 3 files with indirect coverage changes

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

alanlujan91 · 2023-07-25T15:35:41Z

Having trouble setting tests up but this works locally.

alanlujan91 · 2023-07-25T18:21:14Z

@ingydotnet here I am starting the work of building the abstract objects

sbenthall · 2023-07-25T18:21:33Z

Test failures may be due to Python versions (3.8, 3.9, etc.). A lot of these more advanced language features are relatively recent.

sbenthall · 2023-07-25T18:23:32Z

HARK/abstract/tests/consindshk.yml

+      name: m
+      short_name: money
+      long_name: market resources
+      latex_repr: \mNrm


I know how you're imagining using the Latex representation.
But my preference would be to not include it in the PR unless you have some demonstration of how it works ready.
4 different ways to name something for a quick demo seems like a lot....

For now these are filler, I want to throw any non-required keys into an attributes dictionary.

sbenthall · 2023-07-25T18:24:24Z

HARK/abstract/tests/consindshk.yml

+    - !Action
+      name: c
+      short_name: consumption
+      long_name: consumption


Are these fields optional? Can one spill over to the others as a default?
Basically, how can we make these config files lighter weight?

These are optional, only required is name.

sbenthall · 2023-07-25T18:25:36Z

HARK/abstract/tests/consindshk_full.yml

+      long_name: market resources
+      latex_repr: \mNrm
+    - !State
+      name: &name stigma


I don't understand what the ampersands are doing here.
Maybe document that?

ampersands are aliases, since one of the states is also a control and a post-state. I will document this more as I make more progress.

sbenthall · 2023-07-25T18:29:39Z

HARK/abstract/tests/consindshk.yml

+post_states: !PostStateSpace
+  variables:
+    - !PostState
+      name: a


Looks like you've repeated the post_states block twice in this file?

Not sure how @mnwhite feels, but maybe we don't need to draw a firm distinction between states and post states like this.

Or the labels could be inside the variable, not part of the document structure.

Compare:

var_type_1: variables: - !VarTypeClass1 details var_type_2: variables: - !VarTypeClass2 details

with

variables: - !VarTypeClass1 details - !VarTypeClass2 details

The latter is same information, but fewer lines.

sbenthall · 2023-07-25T18:32:01Z

HARK/abstract/variables.py

+        self.shocks = self.variables
+
+
+def make_state_array(


Is this method duplicated?

sbenthall · 2023-07-25T18:32:25Z

HARK/abstract/tests/test_variables.py

+import yaml
+
+import HARK.abstract.variables
+


A test case showing how this data structure could be used in practice would go a long way.

sbenthall · 2023-07-25T18:35:55Z

I think this is a cool demonstration of how PyYAML can be leveraged to make model configuration files in YAML without a custom parser.

The tricky part, as we know, is function definitions.

alanlujan91 · 2023-07-25T18:36:17Z

Test failures may be due to Python versions (3.8, 3.9, etc.). A lot of these more advanced language features are relatively recent.

yep, this might have to be a feature for the future, just getting some initial work done on it

alanlujan91 · 2023-07-25T18:37:31Z

I think this is a cool demonstration of how PyYAML can be leveraged to make model configuration files in YAML without a custom parser.

The tricky part, as we know, is function definitions.

Yes! I was just talking to one of the creators of YAML who was telling me about this https://github.com/yaml/yamlscript

sbenthall · 2023-07-25T18:41:08Z

Mind blown emoji.

One more thing: I'd be keen to see how you would initialize a distribution for a shock directly from YAML.

sbenthall · 2023-07-26T17:24:43Z

HARK/abstract/variables.py

+
+
+@dataclass
+class Auxiliary(Variable):


I agree with Chris that 'auxiliary' is more like a macro.
I wouldn't use 'auxiliary' here, though it makes sense to have this sort of object.

sbenthall · 2023-07-26T17:31:31Z

HARK/abstract/variables.py

+
+
+@dataclass
+class Variable(YAMLObject):


Is there a way to initialize Variables without parsing them from a YAML file?
I.e., a pure python way to create variables?

I'm a little wary of tying the model objects too tightly to the serial format because it can make it tricky to interoperate with other python libraries.

alanlujan91 · 2023-08-07T13:34:29Z

@MridulS thank you for fixing the checks!

Might you be able to help me with the failing test? I'm not sure why !Parameters is not being recognized.

HARK/abstract/variables.py

Co-authored-by: Mridul Seth <mail@mriduls.com>

alanlujan91 added 3 commits July 19, 2023 14:36

Create variables.py

a7ea228

values can be list or array

19ceb72

rename make_states

f41c944

alanlujan91 requested review from sbenthall and mnwhite July 19, 2023 18:45

alanlujan91 added 5 commits July 25, 2023 09:45

implement abstract classes

c523d4b

remove xarray merge for now

ccdf96e

add tests

207696f

update path

918177d

update requirements

376a49a

sbenthall reviewed Jul 25, 2023

View reviewed changes

HARK/abstract/variables.py

self.shocks = self.variables

def make_state_array(

Copy link

Contributor

sbenthall Jul 25, 2023

Choose a reason for hiding this comment

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

Is this method duplicated?

sbenthall reviewed Jul 25, 2023

View reviewed changes

sbenthall reviewed Jul 26, 2023

View reviewed changes

alanlujan91 requested a review from MridulS August 2, 2023 17:44

turn off 3.8, 3.9 tests for now

64e4c09

MridulS reviewed Aug 7, 2023

View reviewed changes

HARK/abstract/variables.py Outdated Show resolved Hide resolved

Update HARK/abstract/variables.py

e4d2850

Co-authored-by: Mridul Seth <mail@mriduls.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Abstract Variables/Symbols #1308

[WIP] Abstract Variables/Symbols #1308

alanlujan91 commented Jul 19, 2023

codecov bot commented Jul 19, 2023 •

edited

Loading

alanlujan91 commented Jul 25, 2023

alanlujan91 commented Jul 25, 2023

sbenthall commented Jul 25, 2023

sbenthall Jul 25, 2023

alanlujan91 Jul 25, 2023

sbenthall Jul 25, 2023

alanlujan91 Jul 25, 2023

sbenthall Jul 25, 2023

alanlujan91 Jul 25, 2023

sbenthall Jul 25, 2023

sbenthall Jul 25, 2023

sbenthall Jul 25, 2023

sbenthall commented Jul 25, 2023

alanlujan91 commented Jul 25, 2023

alanlujan91 commented Jul 25, 2023

sbenthall commented Jul 25, 2023

sbenthall Jul 26, 2023

sbenthall Jul 26, 2023

alanlujan91 commented Aug 7, 2023

		import yaml

		import HARK.abstract.variables



		@dataclass
		class Auxiliary(Variable):



		@dataclass
		class Variable(YAMLObject):

[WIP] Abstract Variables/Symbols #1308

Are you sure you want to change the base?

[WIP] Abstract Variables/Symbols #1308

Conversation

alanlujan91 commented Jul 19, 2023

codecov bot commented Jul 19, 2023 • edited Loading

Codecov Report

alanlujan91 commented Jul 25, 2023

alanlujan91 commented Jul 25, 2023

sbenthall commented Jul 25, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sbenthall commented Jul 25, 2023

alanlujan91 commented Jul 25, 2023

alanlujan91 commented Jul 25, 2023

sbenthall commented Jul 25, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alanlujan91 commented Aug 7, 2023

codecov bot commented Jul 19, 2023 •

edited

Loading