Skip to content

Added a screenshot field to the ProfileData structure #176

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Added a screenshot field to the ProfileData structure #176

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
229af98
Added a screenshot field to the ProfileData structure
mthielma Jul 8, 2025
487ecc0
Added the functionality to call the extract_ProfileData! function wit…
mthielma Jul 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 31 additions & 3 deletions src/ProfileProcessing.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
VolData::Union{Nothing, GeophysicalModelGenerator.GeoData}
SurfData::Union{Nothing, NamedTuple}
PointData::Union{Nothing, NamedTuple}
ScreenshotData::Union{Nothing, NamedTuple}
Copy link
Member

Choose a reason for hiding this comment

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

Why don't you add it as:

 ScreenshotData::Union{Nothing, NamedTuple}=nothing

or

 ScreenshotData::Union{Nothing, NamedTuple}=NamedTuple()

In that case, the tests below are not forced to add an empty NamedTuple() if this is not required

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I get the following error when I use the first suggestion:

ERROR: LoadError: syntax: "ScreenshotData::Union{Nothing, NamedTuple} = nothing" inside type definition is reserved around /Users/mthiel/.julia/dev/GeophysicalModelGenerator/src/ProfileProcessing.jl:9

The same error occurs when I use the empty NamedTuple.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could also solve this issue via multiple dispatch.

Copy link
Member

Choose a reason for hiding this comment

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

true, I overlooked the fact that this is a struct definition. You can add a function like:

ProfileData(VolData, SurfData, PointData, ScreenshotData=nothing) = ProfileData(VolData, SurfData, PointData, ScreenshotData)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added an additional extract_ProfileData! function. Tests now run without including a NamedTuple() argument. Is that what you had in mind?


function ProfileData(; kwargs...) # this constructor allows to define only certain fields and leave the others blank
K = new(true, nothing, nothing, nothing, nothing, nothing, nothing)
Expand Down Expand Up @@ -282,6 +283,31 @@
return nothing
end

### internal function to process screenshot data - contrary to the volume data, we here have to save lon/lat/depth pairs for every screenshot data set, so we create a NamedTuple of GeoData data sets
function create_profile_screenshot!(Profile::ProfileData, DataSet::NamedTuple)
num_datasets = length(DataSet)

tmp = NamedTuple() # initialize empty one
DataSetName = keys(DataSet) # Names of the datasets

for idata in 1:num_datasets
# load data set --> each data set is a single GeoData structure, so we'll only have to get the respective key to load the correct type
data_tmp = DataSet[idata]
if Profile.vertical
x_profile = flatten_cross_section(data_tmp, Start = Profile.start_lonlat) # compute the distance along the profile
data_tmp = addfield(data_tmp, "x_profile", x_profile)

# add the data set as a NamedTuple
data_NT = NamedTuple{(DataSetName[idata],)}((data_tmp,))
tmp = merge(tmp, data_NT)
Copy link
Member

Choose a reason for hiding this comment

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

Probably not important, but this is creating a type instability becuse tmp is changing of type every time you add a new field into it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Isn't the type defined when tmp is initialized as NamedTuple? If not, would there be any possibility to resolve this? As we use this kind of procedure not only in create_profile_screenshot!, but also in create_profile_surface! and create_profile_point!, we could fix it everywhere.

Copy link
Member

Choose a reason for hiding this comment

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

The type of a the variables inside the NamedTuple, as well as its length, are in the type signature of the NamedTuple. So even the types of n1 = (;a=1) and n2 = (;a=2, b=1) are different because length(n1)=1 and length(n2)=2. In your code above tmp is initilised as an empty tuple, and then it changes its type when data_NT is merged. This can be seen running the code below.

function foo(a,b,c)
    tmp = NamedTuple()
    T0 = typeof(tmp)
    println("type of tmp: ", typeof(tmp))
    tmp = merge(tmp, (;a))
    println("type of tmp: ", typeof(tmp))
    tmp = merge(tmp, (;b))
    println("type of tmp: ", typeof(tmp))
    tmp = merge(tmp, (;c))
    println("type of tmp: ", typeof(tmp))
    Tf = typeof(tmp)
    println("T0 == Tf : ", T0 == Tf)
end

a,b,c=1,2,3
julia> foo(a,b,c)
type of tmp: @NamedTuple{}
type of tmp: @NamedTuple{a::Int64}
type of tmp: @NamedTuple{a::Int64, b::Int64}
type of tmp: @NamedTuple{a::Int64, b::Int64, c::Int64}
T0 == Tf : false

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As we do not know the number of fields or datasets beforehand, I don't think we can get around this issue.

else

Check warning on line 303 in src/ProfileProcessing.jl

View check run for this annotation

Codecov / codecov/patch

src/ProfileProcessing.jl#L303 

Added line #L303 was not covered by tests
# we do not have this implemented
#error("horizontal profiles not yet implemented")
end

Check warning on line 306 in src/ProfileProcessing.jl

View check run for this annotation

Codecov / codecov/patch

src/ProfileProcessing.jl#L306 

Added line #L306 was not covered by tests
end
Profile.SurfData = tmp # assign to profile data structure
return
end

### internal function to process surface data - contrary to the volume data, we here have to save lon/lat/depth pairs for every surface data set, so we create a NamedTuple of GeoData data sets
function create_profile_surface!(Profile::ProfileData, DataSet::NamedTuple; DimsSurfCross = (100,))
Expand Down Expand Up @@ -362,18 +388,20 @@


"""
extract_ProfileData!(Profile::ProfileData,VolData::GeoData, SurfData::NamedTuple, PointData::NamedTuple; DimsVolCross=(100,100),Depth_extent=nothing,DimsSurfCross=(100,),section_width=50)
extract_ProfileData!(Profile::ProfileData,VolData::GeoData, SurfData::NamedTuple, PointData::NamedTuple, ScreenshotData::NamedTuple; DimsVolCross=(100,100),Depth_extent=nothing,DimsSurfCross=(100,),section_width=50)

Extracts data along a vertical or horizontal profile
"""
function extract_ProfileData!(Profile::ProfileData, VolData::Union{Nothing, GeoData} = nothing, SurfData::NamedTuple = NamedTuple(), PointData::NamedTuple = NamedTuple(); DimsVolCross = (100, 100), Depth_extent = nothing, DimsSurfCross = (100,), section_width = 50km)
function extract_ProfileData!(Profile::ProfileData, VolData::Union{Nothing, GeoData} = nothing, SurfData::NamedTuple = NamedTuple(), PointData::NamedTuple = NamedTuple(),ScreenshotData::NamedTuple = NamedTuple(); DimsVolCross = (100, 100), Depth_extent = nothing, DimsSurfCross = (100,), section_width = 50km)

if !isnothing(VolData)
create_profile_volume!(Profile, VolData; DimsVolCross = DimsVolCross, Depth_extent = Depth_extent)
end
create_profile_surface!(Profile, SurfData, DimsSurfCross = DimsSurfCross)
create_profile_point!(Profile, PointData, section_width = section_width)

if !isempty(ScreenshotData)
create_profile_screenshot!(Profile, ScreenshotData)
end
return nothing
end

Expand Down
32 changes: 19 additions & 13 deletions test/test_ProfileProcessing.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ prof1 = ProfileData(start_lonlat = (5, 45), end_lonlat = (15, 49))
prof2 = ProfileData(depth = -100)
prof3 = ProfileData(start_lonlat = (5, 45), end_lonlat = (5, 49))
prof4 = ProfileData(depth = -20)
prof5 = ProfileData(start_lonlat = (12, 49), end_lonlat = (12, 45))

# test internal routines to intersect profile with volumetric data:
GeophysicalModelGenerator.create_profile_volume!(prof1, VolData_combined1)
Expand All @@ -80,27 +81,32 @@ GeophysicalModelGenerator.create_profile_point!(prof4, Data.Point, section_width
@test length(prof1.PointData[1].lon) == 13
@test length(prof4.PointData[1].lon) == 445

# test screenshot data
GeophysicalModelGenerator.create_profile_screenshot!(prof5, Data.Screenshot)
@test prof5.SurfData[1].fields.x_profile[1,1,1] == 0


# Test the main profile extraction routines:
extract_ProfileData!(prof1, VolData_combined1, Data.Surface, Data.Point)
extract_ProfileData!(prof2, VolData_combined1, Data.Surface, Data.Point)
extract_ProfileData!(prof3, VolData_combined1, Data.Surface, Data.Point)
extract_ProfileData!(prof4, VolData_combined1, Data.Surface, Data.Point)
extract_ProfileData!(prof1, VolData_combined1, Data.Surface, Data.Point, NamedTuple())
Copy link
Member

Choose a reason for hiding this comment

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

see remark above

extract_ProfileData!(prof2, VolData_combined1, Data.Surface, Data.Point, NamedTuple())
extract_ProfileData!(prof3, VolData_combined1, Data.Surface, Data.Point, NamedTuple())
extract_ProfileData!(prof4, VolData_combined1, Data.Surface, Data.Point, NamedTuple())

extract_ProfileData!(prof1, VolData_combined2, Data.Surface, Data.Point)
extract_ProfileData!(prof2, VolData_combined2, Data.Surface, Data.Point)
extract_ProfileData!(prof3, VolData_combined2, Data.Surface, Data.Point)
extract_ProfileData!(prof4, VolData_combined2, Data.Surface, Data.Point)
extract_ProfileData!(prof1, VolData_combined2, Data.Surface, Data.Point, NamedTuple())
extract_ProfileData!(prof2, VolData_combined2, Data.Surface, Data.Point, NamedTuple())
extract_ProfileData!(prof3, VolData_combined2, Data.Surface, Data.Point, NamedTuple())
extract_ProfileData!(prof4, VolData_combined2, Data.Surface, Data.Point, NamedTuple())

extract_ProfileData!(prof1, VolData_combined3, Data.Surface, Data.Point)
extract_ProfileData!(prof2, VolData_combined3, Data.Surface, Data.Point)
extract_ProfileData!(prof3, VolData_combined3, Data.Surface, Data.Point)
extract_ProfileData!(prof4, VolData_combined3, Data.Surface, Data.Point)
extract_ProfileData!(prof1, VolData_combined3, Data.Surface, Data.Point, NamedTuple())
extract_ProfileData!(prof2, VolData_combined3, Data.Surface, Data.Point, NamedTuple())
extract_ProfileData!(prof3, VolData_combined3, Data.Surface, Data.Point, NamedTuple())
extract_ProfileData!(prof4, VolData_combined3, Data.Surface, Data.Point, NamedTuple())
extract_ProfileData!(prof5, VolData_combined3, Data.Surface, Data.Point, Data.Screenshot)


# Test that it works if only EQ's are provided:
prof4 = ProfileData(depth = -20)
extract_ProfileData!(prof4, nothing, NamedTuple(), Data.Point)
extract_ProfileData!(prof4, nothing, NamedTuple(), Data.Point, NamedTuple())
@test isnothing(prof4.VolData)
@test isempty(prof4.SurfData)
@test length(prof4.PointData[1].depth) == 3280
Expand Down
Loading