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

Separate out transforms from OperatorKernels #46

Merged
merged 20 commits into from
Mar 7, 2022
Merged

Conversation

foldfelis
Copy link
Contributor

@foldfelis foldfelis commented Mar 5, 2022

close #40

TODO:

  • Change the name FourierOperator to OperatorKernel
  • Implement transform family under abstract type AbstractTransform
  • Revise OperatorKernel based on AbstractTransform
  • Extend OperatorKernel to OperatorKernel{<:AbstractTransform}
  • Implement SpectralConv function as a constructor of OperatorKernel{FourierTransform}
  • Test example
  • Update docs

@codecov
Copy link

codecov bot commented Mar 5, 2022

Codecov Report

Merging #46 (4c05564) into master (91e49d0) will increase coverage by 1.55%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #46      +/-   ##
==========================================
+ Coverage   91.78%   93.33%   +1.55%     
==========================================
  Files           5        6       +1     
  Lines          73       90      +17     
==========================================
+ Hits           67       84      +17     
  Misses          6        6              
Impacted Files Coverage Δ
src/NeuralOperators.jl 100.00% <ø> (ø)
src/Transform/fourier_transform.jl 100.00% <100.00%> (ø)
src/model.jl 100.00% <100.00%> (ø)
src/operator_kernel.jl 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 91e49d0...4c05564. Read the comment docs.

@foldfelis foldfelis self-assigned this Mar 7, 2022
src/Transform/fourier_transform.jl Show resolved Hide resolved
"""
AbstractTransform

## Interface
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should be in documentation.

Copy link
Contributor Author

@foldfelis foldfelis Mar 7, 2022

Choose a reason for hiding this comment

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

The doc string will add into the doc automatically I think. Maybe an extra paragraph is needed to discribe how things work? We'll see

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, that's what I mean. Introduction to interfaces should have extra paragraph in doc and should not be in docstring. That way docstring will be too much while the interfaces are many.

src/fourier.jl Outdated
SpectralConv(
ch, modes;
OperatorConv(
ch, modes, transform;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think transform should move before ch or modes.

src/operator_kernel.jl Outdated Show resolved Hide resolved
@foldfelis foldfelis merged commit 07c1653 into master Mar 7, 2022
@foldfelis foldfelis deleted the transforms branch March 7, 2022 08:24
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.

Separate out spectral transforms from operators for easy switching
2 participants