updates to swapi ialirt fitting algorithm #2632
Conversation
1. adds a temporary correction factor based on WIND measurements to be used until the SWAPI L3 science data pipeline is finalized 2. report speed only when the fits fails 3. use `raw count * 16 + 8` instead of `raw count * 16` to avoid truncating to zero counts, which can adversely affect the fits
laspsandoval
left a comment
laspsandoval
left a comment
There was a problem hiding this comment.
Looks good. Just a few things to change. Looking forward to seeing the tests.
| # to be replaced once SWAPI's L3 processing pipeline is finalized | ||
| # this will increase the model count by a factor of e^1, | ||
| # changing the output density by a factor of e^-1. | ||
| density = density * np.exp(1) |
There was a problem hiding this comment.
Move this up to the top so it is easy to change and not calculated every time.
DENSITY_MODEL_SCALE = np.exp(1)
There was a problem hiding this comment.
I agree making this a constant would be good, and then that constant can be brought into the tests. Additionally, it can be changed to 1 later for a quick fix to get back to reality if needed.
Maybe call this TEMPORARY_SCALE_FACTOR or something to indicate it shouldn't be relied upon.
| )[0] | ||
| except Exception as _: | ||
| sol = initial_param_guess.copy() | ||
| sol[1:] = np.nan # report the speed only if the fit fails |
There was a problem hiding this comment.
We are using FILL values instead of nan since downstream we will create cdfs (and that is what SPDF has requested in the past). This file has all fill values listed:
imap_ialirt_l1_variable_attrs.yaml
swapi_pseudo_proton_density:
<<: *default_float32
CATDESC: Pseudo density of solar wind protons
FIELDNAM: swapi_pseudo_proton_density
LABLAXIS: swapi_pseudo_proton_density
UNITS: 1/cm^3
VALIDMIN: 0.0
VALIDMAX: 1000.0
swapi_pseudo_proton_speed:
<<: *default_float32
CATDESC: Pseudo speed of solar wind protons in solar inertial frame
FIELDNAM: swapi_pseudo_proton_speed
LABLAXIS: swapi_pseudo_proton_speed
UNITS: km/sec
VALIDMIN: 0.0
VALIDMAX: 10000.0
swapi_pseudo_proton_temperature:
<<: *default_float32
CATDESC: Pseudo temperature of solar wind protons in plasma frame
FIELDNAM: swapi_pseudo_proton_temperature
LABLAXIS: swapi_pseudo_proton_temperature
UNITS: Kelvin
VALIDMIN: 0.0
VALIDMAX: 50000000.0
default_float32_attrs: &default_float32
<<: *default
FILLVAL: -1.0e31
So in other words add this to the top of your file and use it instead of np.nan.
FILLVAL_FLOAT32 = -1.0e31
| ), | ||
| p0=initial_param_guess, | ||
| )[0] | ||
| except Exception as _: |
There was a problem hiding this comment.
Add a log:
logger.info(f"curve_fit failed for ")
There was a problem hiding this comment.
Can you narrow the Exception down to something like ValueError, or similar so it isn't so broad?
| # Add 8 to avoid having counts truncated to 0 and to avoid counts being systematically too low | ||
| raw_coin_count = raw_coin_count * 16 + 8 |
There was a problem hiding this comment.
do you want to only do this on the minimum side or you do want to do the systematic shift to higher counts too?
raw_coin_count *= 16
raw_coin_count = min(raw_coin_count, 8)| ), | ||
| p0=initial_param_guess, | ||
| )[0] | ||
| except Exception as _: |
There was a problem hiding this comment.
Can you narrow the Exception down to something like ValueError, or similar so it isn't so broad?
| # to be replaced once SWAPI's L3 processing pipeline is finalized | ||
| # this will increase the model count by a factor of e^1, | ||
| # changing the output density by a factor of e^-1. | ||
| density = density * np.exp(1) |
There was a problem hiding this comment.
I agree making this a constant would be good, and then that constant can be brought into the tests. Additionally, it can be changed to 1 later for a quick fix to get back to reality if needed.
Maybe call this TEMPORARY_SCALE_FACTOR or something to indicate it shouldn't be relied upon.
3 participants
Change Summary
Overview
updates to swapi ialirt fitting algorithm
raw count * 16 + 8instead ofraw count * 16to avoid truncating to zero counts, which can adversely affect the fitsUpdated Files
process_swapi.py: made the aforementioned changesTesting
updated
test_process_swapi.pyto account for the density correction, but did not yet create a test for reporting the speed only when the fits fail, nor for the+8shift in counts from the packet. For the latter, perhaps we could add a test using a real packet wherein there are only 3 nonzero counts near the peak.