search

SuperDARN / rst

Radar Software Toolkit (RST)
https://superdarn.github.io/rst/
GNU General Public License v3.0
22 stars 18 forks source link

Update MPFIT library used by lmfit from v1.2 to v1.4 #508

Closed egthomas closed 2 years ago

egthomas commented 2 years ago

I had a look at what might be required to add the more recent lmfit library, and noticed the version of MPFIT included in the RST is about 10 years behind the current version. So this pull request is just an incremental step in that direction to update MPFIT from version 1.2 to 1.4; note that I've retained the 1.2 library in case someone wants to recompile lmfit with that earlier version instead (as was done with astalg?).

ecbland commented 2 years ago

Thanks for making this update @egthomas!

This branch compiles ok, and make_lmfit produces identical results on this branch and develop (should they?)

I think there might be some issues with make_lmfit crashing (I tested with a few different rawacf files and only the first few records get written). This is a separate issue and goes back at least as far as RST4.1. Do you get the same behaviour?

egthomas commented 2 years ago

This branch compiles ok, and make_lmfit produces identical results on this branch and develop (should they?)

@ecbland I think the output should be identical - according to the MPFIT version history, the changes have mostly been bug fixes since v1.2 (https://pages.physics.wisc.edu/~craigm/idl/cmpfit.html):

13 Nov 2010 - version 1.2 - additional documentation, cleanup of mpfit.h
23 Apr 2013 - version 1.3 - add MP_NO_ITER; bug fix mpside=2 when debugging
                             (thanks to M. Wojdyr)
24 Jan 2020 - version 1.4 - bug fixes for 2-sided derivative calculation and
                             potential buffer overflow (thanks T. Larsen @ Oliasoft AS)

I think there might be some issues with make_lmfit crashing (I tested with a few different rawacf files and only the first few records get written). This is a separate issue and goes back at least as far as RST4.1. Do you get the same behaviour?

I honestly haven't used make_lmfit enough to test for that behavior (sorry!).

ecbland commented 2 years ago

note that I've retained the 1.2 library in case someone wants to recompile lmfit with that earlier version instead ... @ecbland I think the output should be identical - according to the MPFIT version history, the changes have mostly been bug fixes

If the output is identical, then I guess there's no harm removing the old mpfit library?

egthomas commented 2 years ago

If the output is identical, then I guess there's no harm removing the old mpfit library?

I don't have a strong feeling one way or the other, and am happy to remove it if necessary.

ecbland commented 2 years ago

If the output is identical, then I guess there's no harm removing the old mpfit library?

I don't have a strong feeling one way or the other, and am happy to remove it if necessary.

Let's remove it then. It just bloats the software and future developers will have no idea why it's there.

egthomas commented 2 years ago

Let's remove it then. It just bloats the software and future developers will have no idea why it's there.

Ok, done. Note that github is now displaying this pull request as a move / renaming to 1.4 even though I deleted the entire 1.2 directory in the latter commit.