search

NOAA-OWP / ngen-cal

Tools for calibrating and configuring NextGen
https://github.com/NOAA-OWP/ngen-cal/wiki
Other
8 stars 15 forks source link

`ngen.cal` performs 1 more iterations than it should #174

Open aaraney opened 3 weeks ago

aaraney commented 3 weeks ago

https://github.com/NOAA-OWP/ngen-cal/blob/6c62306f584cba0ec98f7e15f65aaaa75807c476/python/ngen_cal/src/ngen/cal/search.py#L132 https://github.com/NOAA-OWP/ngen-cal/blob/6c62306f584cba0ec98f7e15f65aaaa75807c476/python/ngen_cal/src/ngen/cal/search.py#L182

I believe should be:

    for i in range(start_iteration, iterations):

Issue found by @ajkhattak (thanks :tada:)

hellkite500 commented 2 weeks ago

If asked to run 100 iterations, iteration 0 is established as an initial evaluation iteration and start_iteration becomes 1. If we want to actually run 100 parameter permutations, then we need a range of 1..100, and since python range is non-inclusive on the end of the range, we use +1 to ensure that we get range(1, 101) which gives 100 parameter permutations.

I don't consider this a bug, but it depends on the expected semantics. The code was originally written based on the semantics I described.