cctbx / cctbx_project

Computational Crystallography Toolbox
https://cci.lbl.gov/docs/cctbx
Other
216 stars 114 forks source link

[Q] Inconsistency in bulk solvent scaling fitting? #643

Open sizmailov opened 3 years ago

sizmailov commented 3 years ago

I have a question about recent change in bulk solvent scaling by @pafonine

In the k_mask_grid_search() we find new k_mask and corresponding k_isotropic in resolution shells via grid search. Next we try to improve r-factor by linear interpolation of k_mask via populate_bin_to_individual_k_mask_linear_interpolation(). Finally we accept k_mask and k_isotropic if they improve r-factor.

The problem is that populate_bin_to_individual_k_mask_linear_interpolation() always use old k_isotropic to compute r-factor. Is there a rationale behind it? Shouldn't it use new k_isotropic instead?

https://github.com/cctbx/cctbx_project/blob/a15fcd9b500288fe471a47b6e32eabf89f78701e/mmtbx/bulk_solvent/scaler.py#L363-L392

sizmailov commented 2 years ago

Sorry for confusion, now I see. Actual old k_isotropic values are effectively ignored by populate_bin_to_individual_k_mask_linear_interpolation() since trial r-factor calculation is done in resolution bin with adjusted overall scale factor (inside of try_scale()). So, please scratch initial question.

Let me ask another one here about same piece of code. Next two snippets are final parts of k_mask_grid_search and bulk_solvent_scaling(). Both functions optimize k_bulk and update k_isotropic in accordance.

https://github.com/cctbx/cctbx_project/blob/a15fcd9b500288fe471a47b6e32eabf89f78701e/mmtbx/bulk_solvent/scaler.py#L388-L392

https://github.com/cctbx/cctbx_project/blob/a15fcd9b500288fe471a47b6e32eabf89f78701e/mmtbx/bulk_solvent/scaler.py#L572-L577

Shouldn't they be exactly the same?