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

Models Genesis Transforms #105

Merged
merged 18 commits into from
Aug 9, 2021
Merged

Conversation

weningerleon
Copy link
Contributor

@weningerleon weningerleon commented Jun 3, 2020

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

Check out this pull request on  ReviewNB

Review Jupyter notebook visual diffs & provide feedback on notebooks.


Powered by ReviewNB

Copy link
Member

@justusschock justusschock left a comment

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
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
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 :)

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
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.

@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
Copy link
Member

@justusschock justusschock left a comment

Choose a reason for hiding this comment

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

cc @mibaumgartner for review

rising/utils/torchinterp1d.py Outdated Show resolved Hide resolved
rising/transforms/functional/intensity.py Outdated Show resolved Hide resolved
rising/utils/torchinterp1d.py Show resolved Hide resolved
@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