-
Notifications
You must be signed in to change notification settings - Fork 0
Model driver #5
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
base: master
Are you sure you want to change the base?
Model driver #5
Conversation
-Interpolation onto depth space is very expensive -Does not need to happen at every timestep
-Rename Module to ModuleWrapper to avoid confustion -Allow user to defer module construction to the wrapper
-Allows for easier access to modules -Check that we're not conflicting with existing model attributes
north/south -Still need to check for uniqueness -Still need to enforce that couplers only get two neighbors
-We can now support arbitrary (non-mixed layer) basin configurations -Still need to validate uniqueness and overloading of couplers
-Coupler can be connected to no more than two modules -Coupler cannot be connected in the same direction multiple times
|
If I try to run example_twocol_plusSO_model.py I get the following error: On a separate note: it seems somewhat awkward to me that neighbors have to be added via the neighbor class. Is that class really necessary? Wouldn't it be nicer if we could just point directly to the module wrappers as neighbors? From what I can see the only information that the neighbor class adds is the direction. Couldn't we just solve that by having left_neighbors and right_neighbors in the module_wrapper? |
|
What I worry we lose with only having single “left” and “right” neighbor is the ability to have zonal and meridional coupling simultaneously. I’m specifically thinking about your “two basin” example.
If we agree that having this flexibility is valuable, I’m also considering moving a chunk of the validation and/or back linking functionality from the module wrapper to the neighbor, where it may live and read more naturally and make some of the operations less awkward.
… On Mar 2, 2020, at 4:06 PM, Malte Jansen ***@***.***> wrote:
If I try to run example_twocol_plusSO_model.py I get the following error:
AttributeError: 'ModuleWrapper' object has no attribute 'validate_neighbor_links'
, thrown from add_module in module.py (l.40). I guess this should probably should be "validate_neighbors" instead?
On a separate note: it seems somewhat awkward to me that neighbors have to be added via the neighbor class. Is that class really necessary? Wouldn't it be nicer if we could just point directly to the module wrappers as neighbors? From what I can see the only information that the neighbor class adds is the direction. Couldn't we just solve that by having left_neighbors and right_neighbors in the module_wrapper?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub <https://github.com/pymoc/pymoc/pull/5?email_source=notifications&email_token=AAACAZWP26QG4KF53WOWUE3RFQNVFA5CNFSM4K5O26D2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOENQ7EAY#issuecomment-593621507>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAACAZUHRKX4VX2TRCT2G5TRFQNVFANCNFSM4K5O26DQ>.
|
|
I agree that we want to allow multiple right or left neighbors, but can't we just do that with lists? |
-Remove neighbor class -Move more validation into the module_wrapper -Track explicit left and right neighbor lists -Short circuit backlinking in a friendlier way
|
@MFJansen I've poked around a bit, and think the current iteration should be a bit cleaner in functionality. The TwoColumn model has a working example, and you can see the API changes there. I think in this case it makes for a more straightforward setup to pass the neighboring module itself through than the code generated key, especially since we'll need that downstream. Thoughts? |
|
@MFJansen I've pushed up a draft of the simplified neighbor model. I still haven't documented or commented the code, but functionality should all be there. One addition worth noting is
Which will let users access modules as |
No description provided.