Unpin gdal and update sdr test expected values

[emlys]

Description

Fixes #905

Checklist

• Updated HISTORY.rst (if these changes are user-facing)
• Updated the user's guide (if needed)
• Tested the affected models' UIs (if relevant)

[emlys]

Not sure if this needs a history note, since it could affect model results, but that's due to a change in GDAL not invest.

[dcdenu4]

Thanks for circling back to this @emlys. Am I right in thinking that these tests would now fail if using an older version of gdal < 3.4.2? If that's the case would we want to set our gdal dependency to gdal>3.4.2?

I think a note in history could be appropriate since it's not an insignificant change in SDR values. Maybe something like:

• Updating SDR test values due to a GDAL improvement in its mode resampling algorithm. [see this issue]

What do you think?

[emlys]

@dcdenu4 I added a history note. I don't think we need to require gdal>=3.4.2 because invest runs just fine with earlier versions. The results are just different. If a user wanted the results from an older version of gdal, I think that should be allowed.

[dcdenu4]

invest runs just fine with earlier versions. The results are just different. If a user wanted the results from an older version of gdal, I think that should be allowed.

Definitely.

I guess I'm just hung up with what would happen if I was working from invest/main but had gdal=3.3.0 and I ran SDR tests? With the updated values, those tests would fail right? Sorry if I'm completely thinking about this wrong...

[emlys]

With the updated values, those tests would fail right?

You're right they would, I guess it's a question of whether requirements.txt should be strict enough that the tests pass? Or just enough that someone can run invest without errors?

[dcdenu4]

Hey @phargogh and @davemfish , this was the other issue we touched on in our last Friday call. As a reminder, this PR updates some SDR tests to pass given gdal>=3.4.2 which made improvements to its resampling algorithms. The question is, should we also pin gdal as a requirement to gdal>=3.4.2 in the process?

[davemfish]

Hey @phargogh and @davemfish , this was the other issue we touched on in our last Friday call. As a reminder, this PR updates some SDR tests to pass given gdal>=3.4.2 which made improvements to its resampling algorithms. The question is, should we also pin gdal as a requirement to gdal>=3.4.2 in the process?

I think it would make our lives slightly easier to require gdal>=3.4.2. Just to avoid the case Doug brought up of building an environment where tests don't pass. Also if we think of the gdal change as a bugfix (it's debate-able, I know), then in general we do want to restrict our requirements to avoid older bugged versions of dependencies, even when they don't raise errors.

I don't think we need to require gdal>=3.4.2 because invest runs just fine with earlier versions. The results are just different. If a user wanted the results from an older version of gdal, I think that should be allowed.

I agree user's should be able to reproduce their results, but I think it's reasonable to say they should install an older version of invest to do so, not just an older version of gdal.

[emlys]

@dcdenu4 I added the requirement gdal>=3.4.2. It's causing a ReadTheDocs build error but I'm not sure if there's anything we can do about that. I'll leave it up to you whether to merge or wait and see if the RTD issue resolves - possibly #1037 will help?

[dcdenu4]

Thanks @emlys ! I think it's okay to go ahead with this with the RTD failing.