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

Start of AdvancedControl refactoring to add global support + more #481

Closed
wants to merge 111 commits into from

Conversation

denised
Copy link
Member

@denised denised commented Oct 5, 2021

What is here:

  • Create a new code directory called meta_model that has things that manage our data but are not domain specific. Things like caching, integration support, and so forth. When we do the data mediator, it will go here.
  • Split the implementation of AdvancedControls in two: ParameterCollection is in meta_model and has the code that handles both global variables and VMA substitution.
  • Draft implementation of ParameterCollection, which simplifies (and drops some features from) VMA substitution, adds global variables, adds support for "additional fields", and makes individual fields able to declare verifiers/initializers.

Things I need to do before we can even run the code:

  • Complete the implementation of AdvancedControls as inheriting from ParameterCollection
  • Fix up all the package references between model and meta_model
  • Fix up the initialization of VMAs so that the solutions define which VMAs go with which parameters.
  • Complete the code path for reading and writing ac objects.

Beyond that:

  • Implement pytest fixtures to extract certain values from expected.zip and set them explicitly.

Other things I want to do as well:

  • Move the list of VMA declarations in __init__.py into a json file, just like the adoptions and tams.
  • Move the "fixed summaries" of VMAs into the VMA metadata
  • Rename AdvancedControls --> ScenarioParameters or something like that.
  • Unify treatment of parameters between old-style models and the new ocean models.

johnpaulalex and others added 2 commits September 30, 2021 19:15
…'t re-export older ac's, and don't overwrite __init__.py (write to an alternate file instead).
@denised
Copy link
Member Author

denised commented Oct 7, 2021

  • Fixed up import statements
  • Renamed DataHandler to JsonMixin, which is a lot more descriptive
  • Moved some tests around

denised and others added 15 commits October 7, 2021 17:04
Changed expected result tester to collect all errors instead of throwing an assertion error on the first one.  This makes the test output even more verbose, so I also added a tool that summarizes it, showing for each solution how many scenarios and which tests are failing.
Expected results gives all errors, and tool summarizes them
Extract 'Low Correction?' entry from excel and return in read_xls(), although it's not written anywhere yet.

Pass bound_correction up from vma extraction (
…uding bound_correction and new description field.

Search for 'Low Correction?' in column Y as well as X.
The second copy of the double-counted adoption must come from ref_base_adoption, not ht, in order to match excel behavior.
Bug fix: ensure that data files have unique names.
Enhancement: delete original data files.
class Scenario has shared class variables that become instance variables when overridden --- but only if you override them.  If you modify them instead, you modify the class variable which then affects all other instances as well.  In this case, it meant that overridden adoption parameters became permanent, and affected subsequent instantiated scenarios.  Detected because walkablecities passed tests by itself, but failed when tested together with other solutions.  I'm amazed that there weren't more issues.

Also deleted a bunch of overrides that were just re-affirming the default value.
@denised denised closed this Aug 24, 2022
@denised denised deleted the globals branch August 24, 2022 07:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants