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

update the slicing of padded arrays #96

Open
amoodie opened this issue Sep 1, 2020 · 1 comment
Open

update the slicing of padded arrays #96

amoodie opened this issue Sep 1, 2020 · 1 comment

Comments

@amoodie
Copy link
Member

amoodie commented Sep 1, 2020

Padded arrays are used throughout the model for water and sediment routing. Two half-baked thoughts, mostly related to speed and simplicity:

  1. are these necessary? I think they are created to be able to safely slice neighbors for any (px, py) pair in the model domain. However, the only place where this should really be needed is directly at the channel inlet, because sediment and water don't reach the domain edge anywhere else. Is there some way to rework the building of routing weights that just pulls from the "base" arrays directly, rather than the padded arrays?
  2. the indexing of padded arrays is usually called with something like: self.pad_stage[px - 1 + 1:px + 2 + 1, py - 1 + 1:py + 2 + 1], which is written as such to account for the padded array itself (the +1 in each index), and the determining of the neighbors (-1 and +2). Note that the neighbors are given as -1 and +2 rather than -1 and +1 because python slicing is exclusive of the endpoint.
    The suggestion here is to eliminate this additional complexity and change slicing to self.pad_stage[px:px + 3, py:py + 3]. With this change, add a section to the developer guide about padded arrays and slicing these arrays, including the explanation above, and a reproduction of the figure below (in matplotlib?). Additionally, make a note in each method's docstring where the slicing was changed, that links to the documentation in the developer guide.

image (2)

@amoodie
Copy link
Member Author

amoodie commented Sep 1, 2020

A simple speed test shows that this may improve slicing speed too:

arr = np.random.uniform(size=(1000, 1000))
s1 = time.time()
for i in np.arange(996):
    for j in np.arange(996):
        _ = arr[i + 1 - 1, j + 2 + 1]
e1 = time.time()
s2 = time.time()
for i in np.arange(996):
    for j in np.arange(996):
        _ = arr[i, j + 3]
e2 = time.time()
addition: 1.1750993728637695
direct: 0.5617256164550781

Not sure this would make a difference in jitted functions though...

@amoodie amoodie added this to the pyDeltaRCM Documentation milestone Jun 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant