Test C implementation of ramp_fitting

[stscijgbot-jp]

Issue JP-3594 was created on JIRA by Nadia Dencheva:

[stscijgbot-jp]

Comment by Kenneth MacDonald on JIRA:

To run the C code, rather than the python code, there is a variable use_c here:

https://github.com/kmacdonald-stsci/stcal/blob/5771276bb2dbe91143aa1cfff3e05cc224396c34/src/stcal/ramp_fitting/ols_fit.py#L664

Set that variable to True.  The variable toggles the code between python and C code.

There are expected differences in outputs when comparing outputs from the python and C code.

There are parts of the code that result in different rounding errors, due to underflows, such as dividing small numbers by small numbers.  This can result in as many as 5% of differences, but they are typically small.  Most differences of this type disappears with tolerances larger than 1.e-4.  Some show up at 1.e-3.  I don't think any show up with 0.01 or larger.  Most of these differences occur in the SCI array.

The other difference that shows up is due to computing integration ramp segments in two separate parts of the code and computing them differently.  This happens far more rarely than the above and typically happens in the variance arrays.  In the files I've tested, when these show up, they only affect a few dozen pixels.

There should not be any differences in the DQ arrays.

[stscijgbot-jp]

Comment by David Law on JIRA:

Awesome, thanks Kenneth MacDonald !

Just confirming that I can install and run your code; I'm presuming this is the branches np_c_ext in your fork of both jwst and stcal.

In one quick test so far I tried processing a MIRI MRS exposure with 188 groups.  SCI arrays on initial inspection show only trivial differences consistent with rounding error.  Runtime of ramp_fit is 2min 26sec with the regular python code, and 7 seconds with the C code.

[stscijgbot-jp]

Comment by Kenneth MacDonald on JIRA:

David Law  There is a np_c_ext branch in JWST as well.  There are two changes in the JWST code.  First, the changes are for the tests, since the C code enforces typing for the RampData arrays; some tests used doubles for arrays, when they should be floats.  Also, there is a change in how the updating of the read noise variance array is updated due to CHARGELOSS flagging.

[stscijgbot-jp]

Comment by Maria Pena-Guerrero on JIRA:

I tested the ramp step on NIRSpec MOS data. The Python code takes about 2 hrs to run, and the C code takes about 10 min. The difference is very small with a mean of about 1.5e-6 and std_dev 3.1e-3 of the SCI arrays. There is indeed no difference in the DQ extensions.

[stscijgbot-jp]

Comment by Nadia Dencheva on JIRA:

Starting a table so it's easier to track results. Please enter all results from now on in the table.

 
||instrument
Mode|| # integ

groups||total run time Python||total run time C||rampfit run time

Python||rampfit run time
C||tested by
notes||Date tested||
|MIRI MRS|188 groups| | |2 min 26 s|  7 s|David Law
trivial differences in SCI|04/05|
|NRS MOS|3 integ
100 groups|~2 hours|10 min| | |Maria Pena-Guerrero
The difference is very small with a mean of about 1.5e-6 and std_dev 3.1e-3 of the SCI arrays. There is indeed no difference in the DQ extensions.|04/09|
|MIR MRS PID 1236 (deep background)|188 groups, 1 int|1062 sec|213 sec|850 sec|12 sec|David Law
Looks good|04/19|
|MIR MRS PID 1247 (bright source/Saturn)|5 groups, 8 ints|44 sec|35 sec|12 sec|1 sec|David Law
Looks good|04/19|
|NIS WFSS PID 1510|10 groups, 1 int|37 sec|24 sec|18 sec|7 sec|David Law
Looks good|04/19|
|MIR MRS PID 1523 (bright pt source)|45 groups, 1 int|86 sec|43 sec|44 sec|2 sec|David Law
Looks good|04/19|
|NRS IFU PID 1591|9 groups, 1 int|47 sec|24 sec|20 sec|2 sec|David Law
Looks good|04/19|
|NRC TSO PID 1952|2 groups, 759 ints|310 sec|102 sec|215 sec|13 sec|David Law
Looks good|04/19|
|NRS MSA PID 2123|32 groups, 3 ints|1290 sec|760 sec|550 sec|20 sec|David Law
Looks good|04/19|
|NRC IMA PID 2736|8 groups, 1 int|68 sec|49 sec|19 sec|1 sec|David Law
Looks good|04/19|
|NRC IMA PID 2739|8 groups, 1 int|41 sec|22 sec|17 sec|2 sec|David Law
Looks good|04/19|
|MIRI Imaging, PID 1248|10 groups, 5 ints| | |19 sec|1.3 sec|Ned Molter|04/22|
|MIRI Imaging, PID 1864|292 groups, 1 int|3900 sec|112 sec|3840 sec|18 sec|David Law
Looks good|04/27|

[stscijgbot-jp]

Comment by David Law on JIRA:

Just updated the table with the results of testing a variety of kinds of data from all 4 instruments.  All look good, negligible differences in SCI and ERR arrays and no differences in the DQ arrays.  ramp fitting runtime improves dramatically, though in some cases this doesn't have an enormous impact on total detector1 runtime because the jump step is now the tallest pole.

Also worth noting that the C code here does not (currently) pull in any of the changes being made for https://jira.stsci.edu/browse/JP-3463, so whatever is done there would need to be added to the C code here.

To me this looks like it should be about ready to merge into the main code so that it's easier for INS teams to test further without having to install branches.

[stscijgbot-jp]

Comment by Ned Molter on JIRA:

I am unable to pip install stcal when I'm on your branch.  I think Nadia Dencheva mentioned something similar today.  I'm using MacOS Ventura 13.6.3 with Apple M2 Max processors.  I'm copying the error message below.

 

 

      building 'stcal.ramp_fitting.slope_fitter' extension
      clang -fno-strict-overflow -DNDEBUG -O2 -Wall -fPIC -O2 -isystem /Users/emolter/anaconda3/envs/py312/include -arch arm64 -fPIC -O2 -isystem /Users/emolter/anaconda3/envs/py312/include -arch arm64 -DNUMPY=1 -I/private/var/folders/4c/gd84fq3972vc4jnbtqmnrrnr0005r1/T/pip-build-env-lyksx69r/overlay/lib/python3.12/site-packages/numpy/core/include -I/Users/emolter/anaconda3/envs/py312/include/python3.12 -c src/stcal/ramp_fitting/src/slope_fitter.c -o build/temp.macosx-11.1-arm64-cpython-312/src/stcal/ramp_fitting/src/slope_fitter.o
      src/stcal/ramp_fitting/src/slope_fitter.c:2032:9: warning: variable 'nan_cnt' set but not used [-Wunused-but-set-variable]
          int nan_cnt;
              ^
      src/stcal/ramp_fitting/src/slope_fitter.c:2637:35: error: expected ')'
          seg->var_p = (pr->median_rate pr->dcurrent) / pden;
                                        ^
      src/stcal/ramp_fitting/src/slope_fitter.c:2637:18: note: to match this '('
          seg->var_p = (pr->median_rate pr->dcurrent) / pden;
                       ^
      src/stcal/ramp_fitting/src/slope_fitter.c:2682:39: error: expected ')'
              seg->var_p = (pr->median_rate pr->dcurrent) / pden;
                                            ^
      src/stcal/ramp_fitting/src/slope_fitter.c:2682:22: note: to match this '('
              seg->var_p = (pr->median_rate pr->dcurrent) / pden;
                           ^
      src/stcal/ramp_fitting/src/slope_fitter.c:2843:39: error: expected ')'
              seg->var_p = (pr->median_rate pr->dcurrent) / pden;
                                            ^
      src/stcal/ramp_fitting/src/slope_fitter.c:2843:22: note: to match this '('
              seg->var_p = (pr->median_rate pr->dcurrent) / pden;
                           ^
      1 warning and 3 errors generated.
      error: command '/usr/bin/clang' failed with exit code 1
      [end of output]
  
  note: This error originates from a subprocess, and is likely not a problem with pip.
  ERROR: Failed building wheel for stcal
Failed to build stcal
ERROR: Could not build wheels for stcal, which is required to install pyproject.toml-based projects ```

[stscijgbot-jp]

Comment by Nadia Dencheva on JIRA:

I get the same error on Monterey. I'll ask Joe to look at the setup file when he's back.

[stscijgbot-jp]

Comment by Kenneth MacDonald on JIRA:

Ned Molter and Nadia Dencheva when did you pull the branch?  I fixed the missing "+" this morning before the stand up and pushed it.

 

https://github.com/kmacdonald-stsci/stcal/blob/f43460e9b68564a5a01376b761ab71a79aaa0da0/src/stcal/ramp_fitting/src/slope_fitter.c#L2641

[stscijgbot-jp]

Comment by Ned Molter on JIRA:

This change fixed the problem for me, thanks. Code runs successfully now.

[stscijgbot-jp]

Comment by Kenneth MacDonald on JIRA:

[^ramp_fit_c_switch.py]

 

^I updated the STCAL code to be able to switch between the C code and the python code programmatically when using the RampFitStep class.  I have attached a test script I used as an example for how to use the JWST RampFitStep class to switch between the C code and the python code.  Notice that I have a directory and file name locally defined, so if you use this script change those variables to match your local directory name and file name.^

[stscijgbot-jp]

Comment by David Law on JIRA:

Technically this isn't 'Done' as testing is the task, but setting to 'Ready for Testing' to make it show up under instrument team RTT searches.

[stscijgbot-jp]

Comment by Kenneth MacDonald on JIRA:

I have updated the RampFitStep spec, so the "OLS_C" can now be selected for the ramp fitting algorithm when using the pipeline:

 

#8503