Skip to content

Fix function compute_scan_angle (QC flags), remove unused argument - #1780

Open
chengdang wants to merge 8 commits into
developfrom
bugfix/cd_compute_scan_angle
Open

Fix function compute_scan_angle (QC flags), remove unused argument#1780
chengdang wants to merge 8 commits into
developfrom
bugfix/cd_compute_scan_angle

Conversation

@chengdang

Copy link
Copy Markdown
Member

Description

This PR fixes compute_scan_angle by:

  • removing the unused argument
  • fixing QC-flag handling
  • updating all instrument converter scripts that call compute_scan_angle

Issue(s) addressed

Resolves issue #1775

Dependencies

None

Impact

None

Checklist

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • I have run the unit tests before creating the PR

@chengdang chengdang self-assigned this Apr 30, 2026
@chengdang chengdang added the needs review Asking others to review - often used for pull requests label Apr 30, 2026
@chengdang

Copy link
Copy Markdown
Member Author

Have not tested it, needs to rebuild ioda

@BenjaminRuston

Copy link
Copy Markdown
Collaborator

needs to rebuild ioda

@chengdang actually for these you don't technically need to rebuild, just copy the new def_jedi_utils.py into the $JEDI_BUILD directory,, the path is in I think lib/python/....

thanks for getting this in and for giving it the attention it needed

@chengdang

Copy link
Copy Markdown
Member Author

@BenjaminRuston
Thanks! That's good to know. I was wondering if I need to re-build. Let me run some tests then.

@BenjaminRuston

Copy link
Copy Markdown
Collaborator

ok yes the hack is to obviously get on the branch and then you have to copy the def_jedi_utils.py file to:

${JEDI_BUILD}/lib/python3.11/pyiodaconv

so that's a shortcut to do a quick test

@chengdang

Copy link
Copy Markdown
Member Author

Thank you @BenjaminRuston
I did a test with the aws file, here is the post-ioda view angle:

image

I think it is working as expected, but more tests with other sensors are probably needed.

Comment thread src/pyiodaconv/def_jedi_utils.py Outdated
Comment thread src/pyiodaconv/def_jedi_utils.py Outdated
# do we need a missing here
ratio = np.empty_like(sensor_altitude)
# Initialize output with missing values
scanang = np.full_like(sensor_altitude, np.nan, dtype=float)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chengdang just one small change, this utility also tried to standardize the int, float and long missing values put into the files... they are globals up at the top

update to float missing from np.nan
@BenjaminRuston
BenjaminRuston marked this pull request as ready for review May 14, 2026 17:31
Comment thread src/pyiodaconv/def_jedi_utils.py Outdated
Comment thread src/pyiodaconv/def_jedi_utils.py Outdated
Comment thread src/pyiodaconv/def_jedi_utils.py Outdated
Comment thread src/pyiodaconv/def_jedi_utils.py Outdated
# do we need a missing here
ratio = np.empty_like(sensor_altitude)
# Initialize output with missing values
scanang = np.full_like(sensor_altitude, float_missing_value, dtype=float)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test are failing, so is the value supposed to be double missing value?

DIFFER : VARIABLE : sensorViewAngle : TYPE : DOUBLE <> FLOAT
DIFFER : TYPES : ATTRIBUTE : _FillValue : VARIABLE : sensorViewAngle : DOUBLE <> FLOAT

@BenjaminRuston

Copy link
Copy Markdown
Collaborator

ok @rajichidamb picked up your other changes and yes got some failures, switching to double to see if the failures change as this may fix those and cause other errors now elsewhere

think we have a situation where some converters used float and other double for the missing value

# do we need a missing here
ratio = np.empty_like(sensor_altitude)
# Initialize output with missing values
scanang = np.full_like(sensor_altitude, double_missing_value, dtype=float)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Setting dtype=float but using double_missing_value? Other python scripts fill the array with 'float32'

 sensor_altitude,
        np.array(f['GeolocationAndFlags']['earth_inc_ang'], dtype='float32'),

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs review Asking others to review - often used for pull requests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants