Skip to content
This repository was archived by the owner on Jan 12, 2024. It is now read-only.

Models Genesis Transforms - #105

Merged
justusschock merged 18 commits into
PhoenixDL:masterfrom
weningerleon:master
Aug 9, 2021
Merged

justusschock merged 18 commits into
PhoenixDL:masterfrom
weningerleon:master

Conversation

@weningerleon

@weningerleon weningerleon commented Jun 3, 2020 •

Copy link
Copy Markdown
Contributor

Short Description

What do you think of this proposition of the models genesis transforms? What kind of tests do you think are necessary?

@review-notebook-app

Copy link
Copy Markdown

Check out this pull request on  ReviewNB

Review Jupyter notebook visual diffs & provide feedback on notebooks.


Powered by ReviewNB

@justusschock justusschock left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

In general this looks good to me :) We may want to change some details (e.g. do a reimplementation of the C++ Part) and we also need to register the newly implemented functions to docs

SEARCHSORTED_AVAILABLE = False


class Interp1d(torch.autograd.Function):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

looking at this repo, we may want to reimplement this part in PyTorch's Python interface (which shouldn't take so long, I'll maybe have a look this weekend).
We would prefer not to have external dependencies :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

With the new torch.searchsorted function, we don't need the external dependency any more :)

Comment thread CODEOWNERS Outdated
/rising/transforms/kernel.py @mibaumgartner
/rising/transforms/spatial.py @justusschock @mibaumgartner
/rising/transforms/utility.py @justusschock @mibaumgartner
/rising/transforms/painting.py @weningerleon

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The CODEOWNERS file is basically not for code ownership (name is a bit misleading), but for automated PR review requests. This means if you register yourself here as an owner, you probably will get automated requests to review PRs for these files.

The actual code ownership is handled based on commits and the associated accounts.

Comment thread rising/transforms/functional/painting.py Outdated
Comment thread rising/transforms/functional/painting.py Outdated
@p-wein p-wein mentioned this pull request Oct 15, 2020
14 tasks
@weningerleon
weningerleon marked this pull request as draft October 22, 2020 11:54
@weningerleon
weningerleon marked this pull request as ready for review October 22, 2020 12:07

@justusschock justusschock left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

cc @mibaumgartner for review

Comment thread rising/utils/torchinterp1d.py Outdated
Comment thread rising/transforms/functional/intensity.py Outdated
Comment thread rising/utils/torchinterp1d.py
@justusschock
justusschock merged commit c836ac9 into PhoenixDL:master Aug 9, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FeatureRequest] Add Equivalent to Models Genesis Transform

3 participants