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

Question about center_slicing #74

Open
ocefpaf opened this issue Jan 19, 2016 · 6 comments
Open

Question about center_slicing #74

ocefpaf opened this issue Jan 19, 2016 · 6 comments
Labels

Comments

@ocefpaf
Copy link
Member

ocefpaf commented Jan 19, 2016

The <var>.center_slicing returns a tuple with slice(s) objects in the form (slice(1, -1, None), slice(1, -1, None)) for each variable.

If I got this right that is trimming the padding, no? If not: how would a node slice be different from a center slice? Shouldn't the trimming be the same? If yes: can we rename this to .trim or something like that?

@ayan-usgs
Copy link
Contributor

@ocefpaf,

Sorry for the delayed response. Things have been crazy busy. 😓

You are correct that is trimming the padding. My understanding of the padding convention as written at https://publicwiki.deltares.nl/pages/viewpage.action?pageId=108953628 would be that the trimming for nodes would be different from that of the center. For example, for a case where <var>.center_slicing is (slice(1, -1, None), slice(1, -1, None), I think the node slicing for that variable would look something like (slice(None, None, None), slice(None, None, None). The reason for is that at its core, the padding specifies whether or not there are trailing edges in a dimension; this then has implications when calculating u and v either at the centers or nodes.

@ocefpaf
Copy link
Member Author

ocefpaf commented Jan 20, 2016

Sorry for the delayed response. Things have been crazy busy. 😓

No rush. I am on "vacation." (That is why I am working on pysgrid 😉)

You are correct that is trimming the padding. My understanding of the padding convention as written at https://publicwiki.deltares.nl/pages/viewpage.action?pageId=108953628 would be that the trimming for nodes would be different from that of the center.

OK. That means the "center" in the name is needed. I would like to suggestion the following:

<var>.trim_center, <var>.trim_node

Also, few users would want to do that alone, most users will perform the average right away. So we could added that as part of .avg_center and .avg_nodemethods (that I am writing 😬)

PS: I am terrible with names, so suggestions are VERY welcome. I just want to make the API consistent and easy to navigate.

@ayan-usgs
Copy link
Contributor

OK. That means the "center" in the name is needed. I would like to suggestion the following:

.trim_center, .trim_node

I think those are splendid names.

Also, few users would want to do that alone, most users will perform the average right away. So we could added that as part of .avg_center and .avg_nodemethods (that I am writing )

Great idea! So these methods, I assume they would be averaging adjacent u and v values on the edges to either the center or a node (e.g. u_center = (u1 + u2)/2, but it wouldn't be doing the vector addition (e.g. resultant_center_velocity = sqrt(u_center**2 + v_center**2)). Just want make sure we're thinking about the same mathematical operations. If not, what are you envisioning?

@ocefpaf
Copy link
Member Author

ocefpaf commented Feb 9, 2016

I don't believe we implemented any of those, right? Or am I missing a few PRs from this repo?

@ayan-usgs
Copy link
Contributor

No, we didn't implement any those. I closed this issue because the question was answered. Could re-open it though.

@ayan-usgs ayan-usgs reopened this Feb 9, 2016
@ocefpaf
Copy link
Member Author

ocefpaf commented Feb 9, 2016

Let's keep it open until we send the proper PR to rename it. I had it ready in my dead laptop and I am re-doing it here. I was just waiting for #78 to stabilize first.

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

2 participants