Submodules for everything! #591
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was done by loading the TurbulenceClosures submodule after the Buoyancy submodule.
Codecov Report
@@ Coverage Diff @@
## master #591 +/- ##
==========================================
+ Coverage 73.14% 73.33% +0.18%
==========================================
Files 70 117 +47
Lines 2011 2014 +3
==========================================
+ Hits 1471 1477 +6
+ Misses 540 537 -3
Continue to review full report at Codecov.
|
ali-ramadhan
commented
Jan 13, 2020
| instantiate(L::NTuple{N, <:DataType}) where N = Tuple(X() for X in L) | ||
| instantiate(L::NTuple{N, <:DataType}) where N = (L[1](), L[2](), L[3]()) # Tuple(X() for X in L) |
There was a problem hiding this comment.
For some reason Tuple(X() for X in L) errors here now (see sample stacktrace below), which is why I changed it to the less general (L[1](), L[2](), L[3]()) which should still work as all the interpolation operators have 3 dimensions and I don't think we're changing this soon.
I mostly just reorganized code so I have no idea why it would error now.
I spent over two hours trying to figure out why Base.Generator errors but couldn't reproduce it in a minimal working example so I'm going to leave it in for now. I can't spend more time on this line.
Unary operations and derivatives [Float32]: Error During Test at /home/alir/Oceananigans.jl/test/test_abstract_operations.jl:243
Test threw exception
Expression: typeof((op(ψ))[2, 2, 2]) <: Number
UndefVarError: I not defined
Stacktrace:
[1] Base.Generator(::Oceananigans.AbstractOperations.var"#5#7", ::Tuple{#s267,#s267,#s267} where #s267<:DataType) at ./generator.jl:32
[2] instantiate(::Tuple{#s267,#s267,#s267} where #s267<:DataType) at /home/alir/Oceananigans.jl/src/AbstractOperations/interpolation_utils.jl:30
[3] interpolation_operator(::Tuple{#s267,#s267,#s267} where #s267<:DataType, ::Tuple{DataType,DataType,DataType}) at /home/alir/Oceananigans.jl/src/AbstractOperations/interpolation_utils.jl:42
[4] _unary_operation(::Tuple{DataType,DataType,DataType}, ::Function, ::Field{Face,Cell,Cell,OffsetArray{Float32,3,Array{Float32,3}},RegularCartesianGrid{Float32,StepRangeLen{Float64,Base.TwicePrecision{Float64},Base.TwicePrecision{Float64}}}}, ::Tuple{DataType,DataType,DataType}, ::RegularCartesianGrid{Float32,StepRangeLen{Float64,Base.TwicePrecision{Float64},Base.TwicePrecision{Float64}}}) at /home/alir/Oceananigans.jl/src/AbstractOperations/unary_operations.jl:27
[5] sqrt(::Tuple{DataType,DataType,DataType}, ::Field{Face,Cell,Cell,OffsetArray{Float32,3,Array{Float32,3}},RegularCartesianGrid{Float32,StepRangeLen{Float64,Base.TwicePrecision{Float64},Base.TwicePrecision{Float64}}}}) at /home/alir/Oceananigans.jl/src/AbstractOperations/unary_operations.jl:92
[6] sqrt(::Field{Face,Cell,Cell,OffsetArray{Float32,3,Array{Float32,3}},RegularCartesianGrid{Float32,StepRangeLen{Float64,Base.TwicePrecision{Float64},Base.TwicePrecision{Float64}}}}) at /home/alir/Oceananigans.jl/src/AbstractOperations/unary_operations.jl:95
[7] top-level scope at /home/alir/Oceananigans.jl/test/test_abstract_operations.jl:243
[8] top-level scope at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.3/Test/src/Test.jl:1107
[9] top-level scope at /home/alir/Oceananigans.jl/test/test_abstract_operations.jl:240
[10] top-level scope at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.3/Test/src/Test.jl:1107
[11] top-level scope at /home/alir/Oceananigans.jl/test/test_abstract_operations.jl:231
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR felt so good!
The internal code base had to be massively refactored thus the size of this PR but I think it's taking the code base in the right direction with submodules for everything, and will make implementing big new features (like vertically stretched grid and MPI) much easier and cleaner.
There were a few breaking changes, e.g.
FaceandCellmoving toOceananigans.Fields. In particular, Oceananigans does not exportday,minute,second, andhouranymore as they conflict with theDatesmodule used by the logger. And it might also confuse users as to why Oceananigans is exporting these common names. They are now exported byOceananigans.Utils.But otherwise, the impact to user scripts is pretty minimal (the examples barely changed).
This should be merged as the next PR ASAP as merging anything else in will probably create some form of merge conflict hell.
Resolves #456
Resolves #495
Resolves #497
Resolves #563