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

replace HARKobject with MetricObject and Model #903

Merged
merged 11 commits into from
Jan 22, 2021
Merged

Conversation

sbenthall
Copy link
Contributor

@sbenthall sbenthall commented Jan 6, 2021

This PR implements a change brought up in issue #612

It replaces HARKobject with two classes, MetricObject, which has the distance metric functionality, and ParameterizedObject, which has the parameter assignment functionality.

The motivations for this are:

  • there is no use case for using the distance functionality on agents; this is only used for elements of a solution
  • the uses of assignParameters in solution elements could be written in the same number of lines using standard Python constructor formatting
  • there are many reasons why parameter assignment for models may need to be handled in a special way (see Equivalency between 2 models/agents.  #612 separate out model definition, solution method parameters, and simulation parameters #660) that is not assigning them to object attributes (which is normally what constructors do). So preserving the ParameterizedObject superclass will make it easier to maker these other changes.
  • 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.

@sbenthall sbenthall changed the title replace HARKobject with MetricObject and ParameterizedObject replace HARKobject with MetricObject and Model Jan 12, 2021
@sbenthall
Copy link
Contributor Author

Latest commits here continue on this trajectory of work:

@sbenthall
Copy link
Contributor Author

Now, Model class has equivalency method __eq__, fixing #612
Asking @MridulS to review

@MridulS MridulS merged commit 8bc1103 into econ-ark:master Jan 22, 2021
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