JP-3834: Update default values for the source finding in tweakreg to match the algorithm

[mcara]

Resolves JP-3834

Closes #9035

This PR changes the default value for source finding to match the default algorithm (IRAFStarFind). Current values .exclude the sharpest stars

Regression tests: https://github.com/spacetelescope/RegressionTests/actions/runs/12528517548

Tasks

• request a review from someone specific, to avoid making the maintainers review every PR
• add a build milestone, i.e. Build 11.3 (use the latest build if not sure)
• Does this PR change user-facing code / API? (if not, label with no-changelog-entry-needed)
  • write news fragment(s) in changes/: echo "changed something" > changes/<PR#>.<changetype>.rst (see below for change types)
  • update or add relevant tests
  • update relevant docstrings and / or docs/ page
  • start a regression test and include a link to the running job (click here for instructions)
    • Do truth files need to be updated ("okified")?
      • after the reviewer has approved these changes, run okify_regtests to update the truth files
• if a JIRA ticket exists, make sure it is resolved properly

news fragment change types...

• changes/<PR#>.general.rst: infrastructure or miscellaneous change
• changes/<PR#>.docs.rst
• changes/<PR#>.stpipe.rst
• changes/<PR#>.datamodels.rst
• changes/<PR#>.scripts.rst
• changes/<PR#>.fits_generator.rst
• changes/<PR#>.set_telescope_pointing.rst
• changes/<PR#>.pipeline.rst

stage 1

• changes/<PR#>.group_scale.rst
• changes/<PR#>.dq_init.rst
• changes/<PR#>.emicorr.rst
• changes/<PR#>.saturation.rst
• changes/<PR#>.ipc.rst
• changes/<PR#>.firstframe.rst
• changes/<PR#>.lastframe.rst
• changes/<PR#>.reset.rst
• changes/<PR#>.superbias.rst
• changes/<PR#>.refpix.rst
• changes/<PR#>.linearity.rst
• changes/<PR#>.rscd.rst
• changes/<PR#>.persistence.rst
• changes/<PR#>.dark_current.rst
• changes/<PR#>.charge_migration.rst
• changes/<PR#>.jump.rst
• changes/<PR#>.clean_flicker_noise.rst
• changes/<PR#>.ramp_fitting.rst
• changes/<PR#>.gain_scale.rst

stage 2

• changes/<PR#>.assign_wcs.rst
• changes/<PR#>.badpix_selfcal.rst
• changes/<PR#>.msaflagopen.rst
• changes/<PR#>.nsclean.rst
• changes/<PR#>.imprint.rst
• changes/<PR#>.background.rst
• changes/<PR#>.extract_2d.rst
• changes/<PR#>.master_background.rst
• changes/<PR#>.wavecorr.rst
• changes/<PR#>.srctype.rst
• changes/<PR#>.straylight.rst
• changes/<PR#>.wfss_contam.rst
• changes/<PR#>.flatfield.rst
• changes/<PR#>.fringe.rst
• changes/<PR#>.pathloss.rst
• changes/<PR#>.barshadow.rst
• changes/<PR#>.photom.rst
• changes/<PR#>.pixel_replace.rst
• changes/<PR#>.resample_spec.rst
• changes/<PR#>.residual_fringe.rst
• changes/<PR#>.cube_build.rst
• changes/<PR#>.extract_1d.rst
• changes/<PR#>.resample.rst

stage 3

• changes/<PR#>.assign_mtwcs.rst
• changes/<PR#>.mrs_imatch.rst
• changes/<PR#>.tweakreg.rst
• changes/<PR#>.skymatch.rst
• changes/<PR#>.exp_to_source.rst
• changes/<PR#>.outlier_detection.rst
• changes/<PR#>.tso_photometry.rst
• changes/<PR#>.stack_refs.rst
• changes/<PR#>.align_refs.rst
• changes/<PR#>.klip.rst
• changes/<PR#>.spectral_leak.rst
• changes/<PR#>.source_catalog.rst
• changes/<PR#>.combine_1d.rst
• changes/<PR#>.ami.rst

other

• changes/<PR#>.wfs_combine.rst
• changes/<PR#>.white_light.rst
• changes/<PR#>.cube_skymatch.rst
• changes/<PR#>.engdb_tools.rst
• changes/<PR#>.guider_cds.rst

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 76.90%. Comparing base (e82bac9) to head (6d0c2d5).
Report is 2 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #9036   +/-   ##
=======================================
  Coverage   76.90%   76.90%           
=======================================
  Files         498      498           
  Lines       45769    45769           
=======================================
  Hits        35199    35199           
  Misses      10570    10570           

Flag	Coverage Δ		*Carryforward flag
nightly	77.39% <ø> (-0.01%)	⬇️	Carriedforward from bb9df64

*This pull request uses carry forward flags. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[mcara]

Many of the unit tests fail because currently starfiner routines from photutils do not include sources having sharpness or roundness equal to limits (sharlo(hi) and roundlo(hi)) for these parameters - see astropy/photutils#1977 and many of unit tests here use perfectly round sources.

[drlaw1558]

Looks good to me; updated parameters are well motivated and fix a recently-reported issue with poor tweakreg performance in a rich field.

[drlaw1558]

Adding a quick note for posterity based on JP-2259 that the range of parameters used by @goudfroo motivating the change to IRAFStarFinder in the first place was sharplo/sharphi = 0.0/3.0 and roundlo/roundhi = 0.0/0.5
(@goudfroo, please correct me if I'm mistaken).

The new defaults set by this PR (0.5/2.0/0.0/0.2) are a much better match to these than the current incorrect defaults (0.2/1.0/-1.0/1.0). Seems best to set defaults to IRAFStarFinder defaults and change parameter ref files for any other instrument-specific tweaks.

[goudfroo]

Correct, @drlaw1558. Just to clarify that these parameter values were found to work well for NIRISS (I didn't also test that for NIRCam or MIRI).

[mcara]

I wanted to ask for an opinion on two things:

1. Set default parameters so that most sources are not excluded by default for both DAO- and IRAF-starfinders? This way there is much less chance of failures due to not finding sources for both algorithms (for example roundness: [-1, 1], sharpness: [0.2, 2]).
2. Update regression tests to explicitly set roundness/sharpness to the old defaults so that there is no need to OKify test results?

[drlaw1558]

@mcara My concern on (1) would be that bad sources might not be rejected with such a permissive range, although I don't have much experience with settings for these parameters myself.
In the immediate term, I think we want to promptly set good defaults for the new IRAFStarFinder usage to make sure that MAST gets good mosaics. In the longer term, what might make most sense from a user perspective would be for these parameter defaults to be linked to the choice of algorithm. I.e., if you choose DAO you get DAO defaults, if you choose IRAF you get IRAF defaults. That way users can swap algorithms without having to remember that these parameters should also be changed. Users or param ref files can still set any custom values they want of course.

[nden]

I agree with David. Also, if someone is changing the default algorithm it's reasonable to expect them to change the default parameters that go with this algorithm.