search

SuperDARN / rst

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

Fitting algorithm field #513

Closed egthomas closed 2 years ago

egthomas commented 2 years ago

This pull request adds a new algorithm field to fitacf-format files, which uses @ecbland's new consolidated make_fit to store an appropriate string for each fitting algorithm and (partially) addresses #479. Similar to #512, the addition of this new field will eventually need to be coordinated with pydarn and pydarnio.

Note the target of this pull request is the feature/make_fit branch rather than develop.

ecbland commented 2 years ago

@egthomas I'm not able to compile this branch. Are there some commits you forgot to push?

The ; is missing on this line...

https://github.com/SuperDARN/rst/blob/ca7d3c1a64dfa46c41e7448a482b793060c7dc6f/codebase/superdarn/src.lib/tk/idl/fitidl.1.5/src/fitidl.c#L115

And I got an error: 

In file included from fitidl.c:34:
fitidl.c: In function ‘IDLCopyFitDataFromIDL’:
fitidl.c:56:34: error: ‘struct FitIDLData’ has no member named ‘algorithm’
   56 |   if (strlen(IDL_STRING_STR(&ifit->algorithm)) !=0)
      |                                  ^~
egthomas commented 2 years ago

@ecbland sorry about that - I was just in the process of fixing those errors when you made your comment. It should compile properly now, and I just verified that the IDL DLM is working. The last thing for me to test is the native IDL support for the new field.

edit the native IDL support seems to be working fine, so there aren't any more changes planned on my end.

egthomas commented 2 years ago

Ok so I changed my mind - one final commit modifies time_plot and field_plot to display the fitting algorithm string (if available) instead of the major and minor revision numbers, eg

20200626 0001 00 cve

If the fitting algorithm is not available from the fitacf file, then the major and minor revision numbers are displayed as usual.

ecbland commented 2 years ago

Thanks for doing this @egthomas! I've tested this with dmapdump, the IDL DLM code and the native IDL code. The changes to time_plot and field_plot also work as described.