Skip to content

Comments

I added multidimensionality functions to the tvrdiff method by callin…#188

Open
mariaprot wants to merge 1 commit intoflorisvb:masterfrom
mariaprot:tvrdiff
Open

I added multidimensionality functions to the tvrdiff method by callin…#188
mariaprot wants to merge 1 commit intoflorisvb:masterfrom
mariaprot:tvrdiff

Conversation

@mariaprot
Copy link

…g it recursively. Some changes might need to be made in terms of making the code less verbose, in particular, replacing moveaxis() with apply_along_axis()

…g it recursively. Some changes might need to be made in terms of making the code less verbose, in particular, replacing moveaxis() with apply_along_axis()
return x_hat, dxdt_hat

#N-d case:
def tvrdiff(x, dt, order, gamma, huberM=float('inf'), solver=None, axis=0):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's keep only one public-facing function. If there needs to be a 1d function, you can indent it and put it inside the outer one.

- **dxdt_hat** (np.array) -- estimated derivative of x
"""

x0 = np.moveaxis(x, axis, 0)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of np.moveaxis with creating a new data array, I think np.apply_along_axis is more automatic and shorter. Strive to do things in as few calls as possible, because each addition becomes something more to parse when people read and manage the code later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants