improve method to supply height for terrain corrections

[craigmillernz]

Current terrain correction method requires the user to set
sites.sample_elevation(dem_file1, output_col="dem_elevation")

before terrain corrections, however this is applied to all grids when computed and it's disconnected from the TerrainCorrectionParameters()

To fix we propose a station_elevation_source parameter in TerrainCorrectionParameters() which has these options.

1. station_elevation_source = "height_ellipsoidal" use the sites.height_ellipsoidal field, but this will nearly always give problems if the DEM is orthometric. perhaps add a warning "is your DEM also ellipsoidal heights ?"

2. station_elevation_source = column name in sites object. If the user has another height field in the sites object, such as height_orthometric, allow them to select that - "height field" - enter the column name from sites object.

3. station_elevation_source = "from_dem" Samples the DEM specified in the TerrainCorrectionParameters() at the station location and uses that value. May need to take care with bathymetry.

If nothing is supplied then throw an error and do not proceed.

Return the actual height used field in the results table.

This should make it more explicit and flexible as to what heights have been used in which part.

Update example scripts and terrain correction docs.

[alkirkby]

I am starting to remember now why we had it this way as I read through this issue.

We need to be careful as we typically apply corrections in different zones and we only want the station height clamped to the most accurate DEM. I think we had it as it is currently, so that the user explicitly clamps the sites prior to applying terrain corrections. I actually think how it is now is good.

If we set it as proposed we need to be careful that the same height is used for each zone.

For the inner zone, we clamp to the DEM we are using for terrain corrections and everything is happy.

For the outer zones, we need to keep the elevation we determined for the inner zone and use that elevation. We don't want to re-clamp to the less accurate DEM. So we will need to ensure the station elevation from DEM is written to a predictable field name, and the user will need to ensure that the station_elevation_source is set correctly to that field name for outer DEMs.

[craigmillernz]

An important issue at the moment is that setting which height is used is a function under a sites object sites.sample_elevation() and disconnected from the terrain correction parameters. At a minimum the height field selection needs to be moved into the TerrainCorrectionParameters() and if dem clamping (i.e. sample_elevation) is required that is also done in the TerrainCorrectionParameters(), not in a sites object as currently.

In some cases you might not want to clamp at all, i.e. if you already have orthometric heights, and the suggested solution gives the user flexibility to choose what is appropriate for each case.

But at the moment you only have one option and it is applied to all DEM in the compute() stage with no flexibility.

I agree that care is required, but the user should be able to make the correct choices which currently are a bit opaque.

[alkirkby]

Ah ok so at the moment you can't tell TerrainCorrectionParameters what height field to use? I agree that that should be an option.
but the sites.sample_elevation() seems good to me as height is really connected to sites.

Happy to have it either way though so if you have a strong preference to the new method then happy to go with it.

[craigmillernz]

telling it which height field to use is done in the
corrector = TerrainCorrector(params=[inner_zone_params, outer_zone_params])
terrain_results = corrector.compute(points=sites, show_progress=True, site_height_field="dem_elevation")
and applied to all DEM with no choice.

Height is connected to sites, but this use of an alternate, DEM sampled elevation, only relates to terrain corrections, so seems more natural for it to be set under terrain corrections. It can still be part of a sites object under the hood, but setting it through the TerrainCorrectionParameters() seems more natural.

[AdrianBenson]

Hi Alison The problem I more that height field can only be specified once as an argument to compute. So the same height will be used for all TC Zones. This provides * granularity - different heights for different zones)\ * Traceability - we have tracked which height field/method was used for each TC zone by preserving it in the parameters/metadata

…

________________________________ From: Alison Kirkby ***@***.***> Sent: Friday, 29 May 2026 14:03 To: GNS-Science/gsolve ***@***.***> Cc: Adrian Benson ***@***.***>; Assign ***@***.***> Subject: Re: [GNS-Science/gsolve] improve method to supply height for terrain corrections (Issue #32) CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe: [https://avatars.githubusercontent.com/u/5335745?s=20&v=4]alkirkby left a comment (GNS-Science/gsolve#32)<#32 (comment)> Ah ok so at the moment you can't tell TerrainCorrectionParameters what height field to use? I agree that that should be an option. but the sites.sample_elevation() seems good to me as height is really connected to sites. Happy to have it either way though so if you have a strong preference to the new method then happy to go with it. — Reply to this email directly, view it on GitHub<#32?email_source=notifications&email_token=AKLRBGND55BMZVK4VZBEFOL45DVYHA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTINJWHE4DINJZGEZKM4TFMFZW63VGMFZXG2LHN2SWK5TFNZ2KYZTPN52GK4S7MNWGSY3L#issuecomment-4569845912>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AKLRBGK5DHTV3OK7WYX57RD45DVYHAVCNFSM6AAAAACZRUN37SVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHM2DKNRZHA2DKOJRGI>. Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS<https://github.com/notifications/mobile/ios/AKLRBGNUEBR2ZMESASWOJSL45DVYHA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTINJWHE4DINJZGEZKM4TFMFZW63VGMFZXG2LHN2SWK5TFNZ2KUZTPN52GK4S7NFXXG> and Android<https://github.com/notifications/mobile/android/AKLRBGKDYQKT32WEJFKX7WL45DVYHA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTINJWHE4DINJZGEZKM4TFMFZW63VGMFZXG2LHN2SWK5TFNZ2K4ZTPN52GK4S7MFXGI4TPNFSA>. Download it today! You are receiving this because you were assigned.Message ID: ***@***.***> Adrian Benson Research Software Support Specialist +64-21-045-0500 1 Fairway Drive, Avalon, Lower Hutt, New Zealand Earth Sciences New Zealand [Earth Sciences New Zealand]<https://earthsciences.nz> The Institute of Geological and Nuclear Sciences Limited and the National Institute of Water and Atmospheric Research Limited joined to become the New Zealand Institute for Earth Science Limited. We are known as Earth Sciences New Zealand. For more information on the Earth Sciences transition click here<https://niwa.co.nz/about-niwa/science-sector-reforms>. Notice: This email and any attachments may contain information which is confidential and/or subject to copyright or legal privilege, and may not be used, published or redistributed without the prior written consent of Earth Sciences New Zealand. If you are not the intended recipient, please immediately notify the sender and delete the email and any attachments. Any opinion or views expressed in this email are those of the individual sender and may not represent those of Earth Sciences New Zealand. For information about how we process data and monitor communications please see our privacy policy<https://earthsciences.nz/privacy-policy>.