Skip to content

Conversation

@dpdutcher
Copy link
Collaborator

This PR does several things related to biasing and bias steps:

  • Adds dev-cfg defaults for bias_dets, both to_rfrac and to_rfrac_range
  • Incorporates Adds dev-cfg defaults for bias_steps and take_bgmap #404 , which adds dev-cfg defaults for take_bias_steps and take_bgmap in the same manner as done for take_iv
  • Makes fitting for the time constant optional, default True.
  • Eliminates the kwarg fit_tmin in favor of determining the tmin from the data itself on a per-channel basis.

Draft PR until I've been able to test all aspects of these changes.

simonscryo and others added 7 commits November 7, 2025 20:43
* take_bias_steps and take_bgmap now work in a similar fashion to take_iv
* Cfg files are generated with empty default dicts for each, which the user can modify
* Moved `dc_voltage` param to take_bias_steps
* Removed redundant docstring of take_bgmap
@dpdutcher dpdutcher requested a review from msilvafe November 11, 2025 16:13
@dpdutcher dpdutcher removed the request for review from msilvafe November 11, 2025 16:56
@dpdutcher
Copy link
Collaborator Author

@dpdutcher
Copy link
Collaborator Author

dpdutcher commented Nov 14, 2025

I've tested bg_map, biasing, and bias_steps on an LF UFM in the Princeton testbed. I've tested analysis of historical bias step data from MF and UHF lab data, and UHF in-field data with and without a spinning HWP.
All worked as expected.

This will change how time constants are computed for all frequencies, but it's strictly for the better. All previous time constants should be recomputed.

@dpdutcher dpdutcher requested a review from msilvafe November 14, 2025 18:16
@dpdutcher dpdutcher marked this pull request as ready for review November 14, 2025 18:20
@kmharrington kmharrington linked an issue Nov 14, 2025 that may be closed by this pull request
@dpdutcher
Copy link
Collaborator Author

dpdutcher commented Nov 18, 2025

I'm noticing now that the way I've implemented rfrac(range) defaults won't work with the way the pysmurf_controller and and sorunlib is written, and that those will also need to be changed.
For example, having defaults for both 'rfrac' and 'rfrac_range' means there's a not a single parameter that's either a tuple or int, deciding between bias_to_rfrac or bias_to_rfrac_range. I'm not sure where that choice should be made.

Copy link
Contributor

@msilvafe msilvafe left a comment

Choose a reason for hiding this comment

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

A couple comments and questions along with a response to your question about rfrac vs rfrac_range inline.

@dpdutcher
Copy link
Collaborator Author

I've implemented and tested my changes of merging bias_to_rfrac and bias_to_rfrac_range. It pulls from the dev cfg correctly, and the user can pass confused calls like bias_to_rfrac(S,cfg, rfrac=(0.3, 0.6)) or bias_to_rfrac_range(S, cfg, rfrac=0.5) and those still work, too.

@msilvafe msilvafe self-requested a review November 21, 2025 19:49
Copy link
Contributor

@msilvafe msilvafe left a comment

Choose a reason for hiding this comment

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

Thanks for testing and addressing comment.

@dpdutcher dpdutcher merged commit 7df34b8 into master Nov 21, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add bias step and bias dets params into the config files

2 participants