search

JCSDA-internal / ioda-converters

Various converters for getting obs data in and out of IODA
8 stars 2 forks source link

Fixed acprofile test failure and some other things. #1467

Closed rmclaren closed 5 months ago

rmclaren commented 5 months ago

Description

Unit test test_iodaconv_prepbufr_ncep_aircftprofiles2ioda was failing due to a subtle problem with the way maxCount was being stored (as reference to a temporary rather than a copy). For some strange reason this actually worked most of the time... Surprised the compiler didn't warn about this (uuuuggg).

On the way to debugging this issue I noticed some other things that are potential problems so here is a list of fixes:

1) ResultSet::analyzeTarget:195 Type of maxCount changed to const auto maxCount from const auto& maxCount. Definitely wrong to have a reference to a temporary return value. 2) ResultSet::analyzeTarget:201-204 Valgrind flagged a potential issue where we were reading past the end of an array. 3) Data::isMissing:210-211 Improved comparison of float values. Wasn't causing an issue but maybe could...

rmclaren commented 5 months ago

@CoryMartin-NOAA This is an example why its important to keep the unit testing with the project, and not some external application. We would never have noticed this if we'd buried the tests somewhere else...

BenjaminRuston commented 5 months ago

excellent @rmclaren so nice to finally button this up... put likely a low estimate on the time but agian thanks for tracking this down