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

API update #38

Merged
merged 6 commits into from Jul 4, 2019

Conversation

Projects
None yet
2 participants
@sixpearls
Copy link
Collaborator

commented Jul 3, 2019

playing with moving tensor API

@sixpearls sixpearls requested a review from ixjlyons Jul 3, 2019

@sixpearls

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 3, 2019

This move introduces some odd calls internally because:

  • numpy does not seem as happy to broadcast to trailing axes
  • to keep the appropriate slices of the __call__ argument C-contiguous for the _bspl implementation.

However, it does seem more consistent for the user:

  • Everything is zero-indexed, including axis for dimension traversal
  • Matches general convention for independent variable axes first. For example, this allows direct interpolation of time series, i.e., from a dataframe make_interp_spline(df.index, df.values)

Some user code will be more ugly, such as needing to pass axis=-1 when stacking to create grids to interpolate or evaluate or selecting output chanels with [..., idx]. These aren't too arduous, especially the latter which is not significantly different from selecting a column from a time-series.

@ixjlyons
Copy link
Member

left a comment

Not a totally thorough review but looks pretty good to me. Maybe update the docstrings for the high level functions as well?

Show resolved Hide resolved ndsplines/ndsplines.py Outdated
self.coefficient_selector_base[i][None, ...],
# Broadcasting does not play nciely with xdim as last axis for some reason,
# so broadcasting manually. Need to determine speed concequences.
self.interval_workspace[i][(slice(0,num_points),) + (None,)*self.xdim],

This comment has been minimized.

Copy link
@sixpearls

sixpearls Jul 4, 2019

Author Collaborator

@ixjlyons Does the azure pipeline run the benchmarks? Or is the best thing for me to run them on my own laptop? I'm curious about how to broadcast this most efficiently.

This comment has been minimized.

Copy link
@sixpearls

sixpearls Jul 4, 2019

Author Collaborator

Or if it matters.

This comment has been minimized.

Copy link
@sixpearls

sixpearls Jul 4, 2019

Author Collaborator

I did a bit of testing and I don't think it will matter too much

This comment has been minimized.

Copy link
@ixjlyons

ixjlyons Jul 4, 2019

Member

Just for the record the benchmarks are only run when building the docs but no big deal to add a task to run them in CI as well.

@sixpearls

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 4, 2019

It looked like all the doc strings are up to date now.

@sixpearls sixpearls merged commit e1bdfd3 into master Jul 4, 2019

7 checks passed

kb-press.ndsplines Build #20190704.2 succeeded
Details
kb-press.ndsplines (Job Linux_Py36) Job Linux_Py36 succeeded
Details
kb-press.ndsplines (Job Linux_Py37) Job Linux_Py37 succeeded
Details
kb-press.ndsplines (Job Mac_Py36) Job Mac_Py36 succeeded
Details
kb-press.ndsplines (Job Mac_Py37) Job Mac_Py37 succeeded
Details
kb-press.ndsplines (Job Windows_Py36) Job Windows_Py36 succeeded
Details
kb-press.ndsplines (Job Windows_Py37) Job Windows_Py37 succeeded
Details

@sixpearls sixpearls deleted the API-fix branch Jul 4, 2019

@sixpearls

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 4, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.