Mm rework lingfeng#21
Conversation
…ed absolute sigma in Linear model
…odel error calculations
… Update trans_positions
Update data fitting logic: Compare observed data points with model parameters. If n_obs < n_params, return fill value for the parameters and infinity for the uncertainties; Otherwise, calculate parameters normally. If n_obs = n_params and absolute_sigma=False, return infinity for uncertainties (Same behavior as scipy curve fit).
…motion models. Update fitting logic: Provide multiple motion models, use the available model with the most number of parameters to fit according to the observed data points (i.e., choose the model with n_obs >= n_params). Removed motion_model_input and default_motion_model for clarity.
…atch_pos at_time into model; Renamed get_ functions into calc_ functions
… and get_batch_pos_at_time into model; Replaced MotionModel.fit_motion_model with MotionModel.fit; Rewrote and accelerated StarTable.fit_velocities with StarTable.fit_motion_model
| if ('me' not in cols) and ('snr' in cols) and (error == True): | ||
| t_ref['me'] = 1.0 / t_ref['snr'] | ||
|
|
||
| # if ('me' not in cols) and ('snr' in cols) and (error == True): |
There was a problem hiding this comment.
We use this option in our microlensing work, and I would like to un-comment it. Does it cause an error in your use case?
There was a problem hiding this comment.
Oh certainly not. But it seems HST still uses 'me' column and in your case 'snr' column? Do we need to differentiate these above in line 649 too?
There was a problem hiding this comment.
Yeah, we need to be able to use 'snr' for Keck data. This has never been an issue with our HST data since those tables already have column headers. Do yours not?
There was a problem hiding this comment.
Reverted back to snr. Need to fix in hst_flystar
…lumns -> meta data -> default value; Added random seed control in bootstrap functions.
| self.meta[param] = value | ||
|
|
||
| if raise_key_error: | ||
| raise KeyError(f"fit_motion_models: Missing required fixed parameter(s) for the motion models used: {', '.join(missing_params)}! Please provide them in fixed_params_dict or as columns in the table.") |
There was a problem hiding this comment.
Can this also check the table metadata for the value too before throwing an error? I think that will be much more common than a full data column
| return | ||
|
|
||
| def shift_reference_frame(self, delta_vx=0.0, delta_vy=0.0, delta_pi=0.0, | ||
| motion_model_dict={}): |
There was a problem hiding this comment.
These functions (class method and standalone) have been broken with these changes. A fix would be appreciated, but I can also make it if needed.
There was a problem hiding this comment.
Do you mean the return type of this function? Actually, I don't call this function anymore in align, so technically I need to remove it in this branch. It is replaced by the motion_model.infer_positions function. Do you still call this function?
There was a problem hiding this comment.
I mean the shift_reference_frame functions. They throw errors because the structure of motion models changed. Which function do you mean you might remove?
There was a problem hiding this comment.
Just updated the shift_reference_frame function, but I don't have a test case for it. I meant to remove the get_star_positions_at_time function, which I replaced with infer_positions
| trans_class=transforms.PolyTransform, | ||
| trans_args=[{'order': 2}, {'order': 2}], | ||
| trans_input=None, | ||
| trans_weighting=None, |
There was a problem hiding this comment.
please revert these weighting/weights keywords for user continuity
There was a problem hiding this comment.
Reverted the align argument keywords (but kept the attribute name inside the class)
…mbine_lists; Added support for artificial star motion model fitting with x/y out (index 1).
…flystar into mm_rework_lingfeng
…eLab/flystar into mm_rework_lingfeng
|
Why the name change? It is strongly preferred to keep backwards compatibility and the function API.On Jul 7, 2026, at 1:35 PM, Wei Lingfeng ***@***.***> wrote:
@wei-lingfeng commented on this pull request.
In flystar/startables.py:
- if column in self.colnames: self.remove_column(column)
-
- # Update self
- for column in columns:
- column_array = MaskedColumn(np.ma.zeros(N_stars), dtype=float, name=column)
- column_array[select_stars] = vel_result[column]
- column_array[select_stars][~detected_in_all_epochs] = np.nan
- column_array.mask[select_stars] = ~detected_in_all_epochs
- # Mask unselected indices
- column_array.mask[~np.isin(np.arange(N_stars), select_stars)] = True
- self[column] = column_array
-
- if return_result:
- return vel_result
- else:
- return
def shift_reference_frame(self, delta_vx=0.0, delta_vy=0.0, delta_pi=0.0,
motion_model_dict={}):
Just updated the shift_reference_frame function, but I don't have a test case for it. I meant to remove the get_star_positions_at_time function, which I replaced with infer_positions
—Reply to this email directly, view it on GitHub, or unsubscribe.Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>
|
|
@jluastro This is a new function in the mm_rework branch only, so I don't think it will affect backward compatibility until the merge is complete and pushed to dev or main. Plus, the API has drastically changed from mm_rework to make things faster, which this pull request is meant to do. mm_rework_lingfeng -> mm_rework -> public dev/main. |
3 participants
No description provided.