Skip to content

Check that self%perturbation is allocated before trying to apply perturbations#29

Merged
dougiesquire merged 1 commit intomasterfrom
gcc
Feb 26, 2026
Merged

Check that self%perturbation is allocated before trying to apply perturbations#29
dougiesquire merged 1 commit intomasterfrom
gcc

Conversation

@dougiesquire
Copy link
Collaborator

This PR adds a check to libforcing/src/forcing_field.F90::forcing_field_apply_perturbations to ensure that that self%perturbation is allocated before trying to apply perturbations.

This prevents errors like:

Program received signal SIGSEGV: Segmentation fault - invalid memory reference.

when running with GCC-compiled binaries

…urbations

This prevents 'invalid memory reference.' errors when using GCC
@dougiesquire dougiesquire self-assigned this Feb 24, 2026
@dougiesquire dougiesquire changed the title Check that self%perturbation is allocated before trying to apply perturbations Check that self%perturbation is allocated before trying to apply perturbations Feb 24, 2026
@dougiesquire
Copy link
Collaborator Author

@anton-seaice could please review.

This allows building and running ACCESS-OM2 with GCC (see here) and doesn't change answers with oneapi (see here)

@anton-seaice anton-seaice self-requested a review February 26, 2026 04:24
@anton-seaice
Copy link
Collaborator

anton-seaice commented Feb 26, 2026

I'm sure i've over thought this

Here:

call self%core%get_child(field_jv_ptr, "perturbations", perturbation_list, found)
if (.not. found) then
return
endif
num_perturbations = self%core%count(perturbation_list)
allocate(field_ptr%perturbations(num_perturbations))

if num_perturbations is 0, then it should return (and not allocate perturbations)

The change lined in this PR could just be

if (.not. allocated(self%perturbations)) then

@anton-seaice
Copy link
Collaborator

We almost need a config / repro test with some perturbations included

@dougiesquire
Copy link
Collaborator Author

I'm possibly not understanding what you're saying, but I think:

allocate(field_ptr%perturbations(0)) 

will still allocate the array, but it's size will be zero.

To be clear, I didn't add the check that size(self%perturbations) == 0.

@anton-seaice
Copy link
Collaborator

Yeah - that's right. But why allocate an array that's not needed? Feels neater just not to allocate it and treat all cases of having no pertubations to apply the same ?

@dougiesquire
Copy link
Collaborator Author

I think I'm fine with what I currently have. It's less code changes and, more importantly, it's what I've tested and I'm not that interested in spending more time on this 😅

@anton-seaice
Copy link
Collaborator

Yeah - its also probably lowest risk

Copy link
Collaborator

@anton-seaice anton-seaice left a comment

Choose a reason for hiding this comment

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

Thanks Dougie - it looks like if no peturbations section is provided in the forcing.json then this array isn't allocated, so this make sense

@dougiesquire dougiesquire merged commit 6fbf94c into master Feb 26, 2026
4 checks passed
@dougiesquire dougiesquire deleted the gcc branch February 26, 2026 10:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants