Fix computation of Stratigraphic Column

[jhidding]

No description provided.

[jhidding]

From all of this, it is still not clear to me where the error is!

[jhidding]

Please, don't change existing files like this. It will break other stuff.

[jhidding]

Please, don't copy paste files with their Entangled annotations intact.

[jhidding]

Same here, what are you trying to achieve?

[jhidding]

A unit test should stand on its own legs. Here you depend on some previous simulation having been run.

The error is not in writing out the CSV, so don't test for that. There are two ways to go about this:

1. run a small simulation with HDF5 output (make it 1d) and write the output to a temporary directory (see

   CarboKitten.jl/test/ExportSpec.jl

   Lines 90 to 108 in b6a5036

   	mktempdir() do path
   	spec = CSV(GRID_LOCATIONS1,
   	:sediment_accumulation_curve => joinpath(path, "sac.csv"),
   	:age_depth_model => joinpath(path, "adm.csv"),
   	:stratigraphic_column => joinpath(path, "sc.csv"),
   	:water_depth => joinpath(path, "wd.csv"),
   	:metadata => joinpath(path, "metadata.toml"))
   	data_export(spec, HEADER1, DATA1)
   	for f in values(spec.output_files)
   	@test isfile(f)
   	end
   	metadata = TOML.parsefile(spec.output_files[:metadata])
   	@test IdDict(Symbol(k) => v for (k, v) in metadata["files"]) == spec.output_files
   	@test length(metadata["locations"]) == 3
   	adm = read_csv(spec.output_files[:age_depth_model], DataFrame)
   	rename!(adm, (n => split(n)[1] for n in names(adm))...)
   	@test adm == ustrip(extract_sac(HEADER1, DATA1, GRID_LOCATIONS1) |> age_depth_model)
   	end

   for an example).
2. find a way of isolating the problem without running a full model, or having the output in memory.

If you like, you can make the stratigraphic_column function more testable by creating a version that only takes deposition and disintegration, and not the header and data instances that come from reading HDF5 files.

[jhidding]

this sum statement doesn't do anything

[jhidding]

these quantities were never meant to be equal.

From what I understand, the stratigraphic column gives you the amount and type of sediment for each time step (basically: production corrected for later erosion), whereas the sediment accumulation curve is an integral quantity (slight modification of the age depth model).

See

CarboKitten.jl/src/Export.jl

Line 174 in b6a5036

age_depth_model(sac::Vector{T}) where {T} = sac |> reverse |> accumulate(min) |> reverse

to see how ADM and SAC are related.

[jhidding]

if this is to be a unit test, it should be encapsulated in a @testset block as follows:

@testset "CarboKitten.Export.stratigraphic_column" begin

...

end

[xyl96]

pushed with assertions. It fails at : sum(sc[ts_down, :]) + sum(acc) == sum(previous_sc). Where previous_sc is sc[ts_down,:] before acc substraction.

[jhidding]

You should use isapprox or unicode ≈ there. These are floating-point values.

[jhidding]

Does it still fail? If so, what does the data look like and when does it fail?

[xyl96]

still fails, but not complaining any assertion this time.

[EmiliaJarochowska]

I'm trying to figure out where we are with this problem. @jhidding are you satisfied with the test proposed by @xyl96 ?

[jhidding]

@xyl96 Ok, in principle now we have a self-contained test, but it doesn't suffice as a unit test, because it takes a few minutes to run, and it doesn't hone in on the problem at hand.

We only need a single column of the generated data. What I would do:

• find a column where the discrepancy is high, preferably in a single facies.
• plot both the disintegration and the the deposition as a function of time.
• is there anything that strikes you as odd? Try to create some artificial curves that still show the same problem.
• reproduce the issue with only a given disintegration, deposition and the stratigraphic_column function.

[xyl96]

the example of test and code to plot is here: examples\StraDiscrep_test\TestSCdiscrep.jl

[EmiliaJarochowska]

My understanding is now that @xyl96 should abandon ship and only move the useful parts of the code to #139 and then sink this branch and PR?