Fix array out of bounds in ectrans-benchmark

[DJDavies2]

Addresses #83. I'm not sure what is suposed to happen here; when nfld is 0 the 3rd dimension of the array zspsc3a is 0 and so referencing element 1 is out of bounds. I have fixed this here by skipping that subroutine call when nfld is 0 but there may be a better solution.

[samhatfield]

I'm glad you found this, but I think the solution is to enforce positive definiteness for nfld. There are a few other places where this is assumed which would also have to be modified if we wanted to permit a value of 0. The variables zspsc3a and zgp3a don't make sense if there are no non-wind 3D fields, for example. I'm also not sure what ecTrans is doing when you pass an array that looks like this:

zspsc3a => sp3d(:,:,3:2)

to the PSPSC3A argument of INV/DIR_TRANS which I believe is mandatory.

Is there a specific reason you want to benchmark with no non-wind 3D fields?

[DJDavies2]

In terms of this:

zspsc3a => sp3d(:,:,3:2)

it results in zspsc3a being a zero sized array so it depends what the rest of the code does with that. Accessing element 1 (or any other element) of dimension 3 isn't allowed.

Beyond that I'm probably the wrong person to ask about the why of this. I'm happy to implement alternative solutions though.

[samhatfield]

Fair enough. If you don't have a need to benchmark ecTrans with nfld = 0, then I suggest you close this PR and I'll put together some validation code so nfld < 0 is no longer permitted.

[DJDavies2]

I don't have the need to benchmark it myself, I am just running the standard ctests. I think the ctests should work? If nfld = 0 is invalid then is this a problem with the ctest itself?

[samhatfield]

Oh I understand now. Sorry, I see now you were referring to this test. Hmm, I wonder why we have that test at all. @wdeconinck could you enlighten us? (Might take a while to reply as he's away.)

[wdeconinck]

I think the reasoning is explained in the ectrans-benchmark file:

! 2) A Multiple "3d" fields are transformed and can be disabled with "--nfld 0"

The change looks a good step in the right direction. Wondering if it's the only problem though, as @samhatfield questions.

[samhatfield]

It's certainly not. To be safe we would have to wrap all references to zspsc3a in the if (nfld > 0) then check. We would also need to introduce more calls to inv_trans and dir_trans without this argument provided. This is adding additional complexity to what is only supposed to be a simple benchmark. Perhaps we should just forget about the case with no fields, as it's a bit contrived anyway. Any strong thoughts there @wdeconinck?

[wdeconinck]

As a benchmark it is not useful with --nfld 0, but for testing it is still a useful code path.

[samhatfield]

This PR isn't finished yet. We still need to wrap all references to zspsc3a within an if block like the one in this PR's change.

[samhatfield]

@DJDavies2 - could you make these changes in your branch?

We need similar changes here and here.

[DJDavies2]

@DJDavies2 - could you make these changes in your branch?

We need similar changes here and here.

I have updated the branch.