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

Feature: Transforms #43

Merged
merged 12 commits into from
Feb 22, 2024
Merged

Feature: Transforms #43

merged 12 commits into from
Feb 22, 2024

Conversation

JakobEliasWagner
Copy link
Collaborator

@JakobEliasWagner JakobEliasWagner commented Feb 15, 2024

Description

Transforming tensors in the setting of neural operators can have many reasons. Transformations can improve model performance, enhance generalization, enable varied input size inputs, facilitate feature extraction, reduce overfitting, improve computational efficiency, or much more.
Currently these transformations need to be applied manually and are dependent on the implementation of the end user. The result is difficulty in implementing correct transformations and especially chained transformations are difficult to apply. This issue aims to introduce a base transformation class for all tensors.

Which issue does this PR tackle?

How does it solve the problem?

  • Introduces a transform base class.
  • Introduces a Compose class which executes multiple transformations sequentially.

How are the changes tested?

  • Added tests for backward and forward pass of Transform with dummy classes.
  • Added tests for backward and forward pass of Compose with dummy classes. The order of the applied transformations proof that the order in which they are called in backward and forward is correct.

Checklist for Contributors

  • Scope: This PR tackles exactly one problem.
  • Conventions: The branch follows the feature/title-slug convention.
  • Conventions: The PR title follows the Bugfix: Title convention.
  • Coding style: The code passes all pre-commit hooks.
  • Documentation: All changes are well-documented.
  • Tests: New features are tested and all tests pass successfully.
  • Changelog: Updated CHANGELOG.md for new features or breaking changes.
  • Review: A suitable reviewer has been assigned.

Checklist for Reviewers:

  • The PR solves the issue it claims to solve and only this one.
  • Changes are tested sufficiently and all tests pass.
  • Documentation is complete and well-written.
  • Changelog has been updated, if necessary.

@JakobEliasWagner JakobEliasWagner self-assigned this Feb 15, 2024
@JakobEliasWagner JakobEliasWagner added the enhancement New feature or request label Feb 15, 2024
@JakobEliasWagner
Copy link
Collaborator Author

@samuelburbulla The current state of this pullrequest also includes backward/undo implementations for the transformations. This can only be done for bijective mappings. There are many non-bijective mappings like cropping, random noise, limiting. In cases where this is not possible the Transform raises a warning. I think that this undo-mapping is still of use in operators especially when constraining them with physics-induced losses. However this would mean that this limits the type of "valid" transformations to those in which the lost information does not pollute the results. Do you think this reverse-mapping should be kept?

src/continuity/transforms/transform.py Outdated Show resolved Hide resolved
tests/transforms/fixtures.py Show resolved Hide resolved
src/continuity/transforms/transform.py Outdated Show resolved Hide resolved
tests/transforms/test_transform.py Outdated Show resolved Hide resolved
Copy link
Collaborator Author

@JakobEliasWagner JakobEliasWagner left a comment

Choose a reason for hiding this comment

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

Adapted to requested changes. Need some further clarification on whether it is okay to remove the UserWarning Filter.

@samuelburbulla samuelburbulla merged commit 3e0a5b4 into main Feb 22, 2024
8 checks passed
@samuelburbulla samuelburbulla deleted the feature/transforms branch February 22, 2024 08:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants