-
Notifications
You must be signed in to change notification settings - Fork 2
Core model architecture refactor / modernization 🎉 #18
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
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…maining 2-dimensional
This was referenced Sep 15, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes:
shared.types.Variableis used to represent all constants, state variables, and intermediate calculation "dynamic" variables. These are "registered" to a given model (see TSM module for examples indynamic_variables.py,state_variables.py,static_variables.py. TheVariable.useattribute determines whether it is dynamic, state, or static. This will change how it is treated.Modelwill be inherited by all sub-modules and powers the underlying workflow. One can override theModel.__init__function for a sub-module (i.e.,tsm.model.EnergyBudget.__init__()) as long assuper().__init__()is called at the end. Withbase.Modelfully instantiated, all functionality will work!Modelinstance containsModel.dataset, which is axarray.Datasetthat will act as the main object for a model. On can init aModelinstance using a pre-existing dataset via theModel(hotstart_dataset=xr.Dataset(...)) pattern. Upon everytime step,Model.datasetis appended.sorter.pyhandles the sorting of variables intoModel.computation_order. This is the order of calculations that will be run on each timestep increment before appending to the pre-existingModel.dataset. The advantage here is that we only have to look at one place to debug a new model, as it will prevent you from even trying to run it if all the variables can't be calculated.__init__function and useModel.register_variable()and/orModel.unregister_variable()as necessary. Using these functions tells the sorter to re-calculateModel.computation_orderas needed.Unfinished work:
xarray.apply_ufuncfor non-math computations. For the Reynolds Number calculation, there is if/else logic, that isn't as easily vectorized. Two functions are affected and commented out with a "TODO" comment! TSM will not be functional until this is resolved. (I see one way to do it, but I want to look for a more elegant solution).test_5_tsm_answers.py.Advantages:
base.Modelcontains all the code that executes a time step. Therefore, we can focus performance tweaks on one location.Variableclasses, and not re-invent the wheel for each sub-module. Any module specific logic will be verified before a run if allowed via recalculatingModel.computation_orderafter changes to the registered variables are made.Test coverage stats:
Test coverage refers to the % of lines of codes triggered by our test suite. To see this, run
pytest --cov. This requires an extension that is included in theenvironment.yml, so if you haven't cloned that environment to that first.base.py: 81%shared/types.py: 100%sorter.py: 96%utils.py: 61%