NetCDFOutputWriter: new features and bug fixes #823
Conversation
Codecov Report
@@ Coverage Diff @@
## master #823 +/- ##
==========================================
+ Coverage 70.72% 71.38% +0.66%
==========================================
Files 188 189 +1
Lines 5113 5270 +157
==========================================
+ Hits 3616 3762 +146
- Misses 1497 1508 +11
Continue to review full report at Codecov.
|
| @@ -90,7 +90,7 @@ example, to set up a 1D model with $N_z$ grid points | |||
| julia> grid = RegularCartesianGrid(topology=(Flat, Flat, Bounded), | |||
| size=(1, 1, 90), extent=(1, 1, 1000)) | |||
| RegularCartesianGrid{Float64, Flat, Flat, Bounded} | |||
| domain: x ∈ [0.0, 1.0], y ∈ [0.0, 1.0], z ∈ [-1000.0, 11.111111111111086] | |||
| domain: x ∈ [0.0, 1.0], y ∈ [0.0, 1.0], z ∈ [-1000.0, -2.471452993948034e-14] | |||
There was a problem hiding this comment.
Hmmm yeah it's not pretty but that's just floating point arithmetic I guess. Interestingly, in this case
julia> grid = RegularCartesianGrid(topology=(Flat, Flat, Bounded),
size=(10, 10, 10), x=(0, 1), y=(0, 1), z=(0, 1))
RegularCartesianGrid{Float64, Flat, Flat, Bounded}
domain: x ∈ [0.0, 1.0], y ∈ [0.0, 1.0], z ∈ [-1.6190752442450216e-17, 0.9999999999999999]
topology: (Flat, Flat, Bounded)
resolution (Nx, Ny, Nz): (10, 10, 10)
halo size (Hx, Hy, Hz): (1, 1, 1)
grid spacing (Δx, Δy, Δz): (0.1, 0.1, 0.1)it's because
julia> -0.1 + 1.0 + 2*1.0 * 0.1
1.1
julia> -0.1 + (1.0 + 2*1.0 * 0.1)
1.0999999999999999so we could probably fix it by changing
xF₊, yF₊, zF₊ = XF₊ = @. XF₋ + total_extent(topology, halo, Δ, L)to something like
xF₊, yF₊, zF₊ = XF₊ = @. right_endpoint(topology, XF₋, halo, Δ, L)
| global_attributes=Dict(), output_attributes=Dict(), dimensions=Dict(), | ||
| clobber=true, compression=0, slice_kw...) | ||
| clobber=true, compression=0, include_halos=false, verbose=false, slice_kwargs...) |
There was a problem hiding this comment.
This keyword argument could also be with_halos. We have a function with_tracers (this might be the only with_* currently); I had imagined we might have more with_* in the futures (eg a with_boundary_conditions function for Fields. So we could use that kind of nomenclature here.
There was a problem hiding this comment.
This looks great. The only change that might be considered it to change include_halos to with_halos, if we want to start promoting the pattern with_* in the API.
On the default choice to omit halos: I think (but am not completely sure) that this is what most users prefer when interacting with data as arrays, for convenience. We have gotten a bit of feedback on this question.
I do hope that some day we have abstractions for interacting with data as fields, rather than arrays. At that point, it may make sense to have halos saved by default. The reason is that without halos, fields cannot be differentiated or interpolated across boundaries correctly.
On the other hand, omitting halos is convenient for common data analysis tasks, such as plotting or the calculation of simple statistics (by "simple", this means statistics that do not involve differentiating or interpolating fields across boundaries, since this cannot be done correctly when halos are omitted). For the sake of this convenience, I think it makes sense to omit halos when data is being interacted with as arrays.
This PR adds new features to the
NetCDFOutputWriterand fixes some bugs.verboseflag (default isfalse) in case you want to track what's being to disk and how long it takes for each output to be computed.include_halosflag (default isfalse) in case you want to use the full grid including halos to write fields and custom output. Added new tests for this.grid.xC, etc.). This has been fixed.z ∈ [-1.0, 0.0625]should bez ∈ [-1.0, 0.0]. This has been fixed now.I'd like to start using these features in LESbrary.jl (and in PR #572) so I'll tag v0.33.0 once this PR is merged.
Resolves #683