-
Notifications
You must be signed in to change notification settings - Fork 168
Cleaning up Field state #1989
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
Cleaning up Field state #1989
Conversation
VeckoTheGecko
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One thing I'm a bit concerned about is managing state that is specific to the (un)structured Fields and their implementations within Parcels. It would be good to keep Field "grid agnostic", meaning grid specific logic is managed within the grid object1. The problem with this is that, for unstructured grids, we can't just add data to uxarray.Grid since we don't have control over that class, so we might need to go for some other mechanism2.
@fluidnumerics-joe is there any state that you think you'll need to save for unstructured grids in Parcels (i.e., that doesn't make sense to put in uxarray upstream?). I haven't changed the n_face property of Field yet - would be great to know your thoughts on that.
Footnotes
-
This helps with abstraction: making the code easier to read and no need for a lot of "if" statements to define variables in one case for unstructured or another for unstructured ↩
-
This could either be caching (e.g.,
lrucache) for items not specific to a field object, or by adding an objectunstructured_stateto theFieldclass 3 ↩ -
I love footnotes ↩
get_spatial_hash() returns the cached spatial hash be default anyway
Contributes to #1965
The adapter now wraps a Grid instance instead of inheriting from it. This provides better separation of concerns and makes the adapter's purpose more explicit - it's adapting an existing grid rather than extending it. It also allows for more flexibility in adapting any grid instance, regardless of how it was constructed.
4fc2322 to
dbbc290
Compare
This PR cleans up the state on the
Fieldobject (building on #1946)This PR implements the following changes:
_location_vertical_location_spatialhash_gtype_lonlat_minmaxgridadapterGridAdapterto use composition_core.utilssubpackage to put helper functions etcFor context, here is the discussion I had with @fluidnumerics-joe
field-testingwhich will go intov4-dev)