Re: [cctbxbb] [git/cctbx] master: re-defining the shift/est convergence limit in terms of crystallographic rather than independent parameters to make it easier to relate to the convergence reported in the CIF; also caching the shifts for further analysis and reporting; (9339fd5f7)
I'm worried about making changes like this to a fundamental toolbox component like normal equations solving. We use this for processing xfel data, and would like the opportunity to test any changes through a pull request, prior to having them committed to the main branch. Nick Sauter Nicholas K. Sauter, Ph. D. Senior Scientist, Molecular Biophysics & Integrated Bioimaging Division Lawrence Berkeley National Laboratory 1 Cyclotron Rd., Bldg. 33R0345 Berkeley, CA 94720 (510) 486-5713 On Wed, Sep 5, 2018 at 5:29 AM, CCTBX commit < [email protected]> wrote:
Repository : ssh://g18-sc-serv-04.diamond.ac.uk/cctbx On branch : master
------------------------------
commit 9339fd5f7d240f0d6f85dd57ce42e796d19608f1 Author: pcxod
Date: Wed Sep 5 13:29:29 2018 +0100 re-defining the shift/est convergence limit in terms of crystallographic rather than independent parameters to make it easier to relate to the convergence reported in the CIF; also caching the shifts for further analysis and reporting;
------------------------------
9339fd5f7d240f0d6f85dd57ce42e796d19608f1 scitbx/lstbx/normal_eqns_solving.py | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/scitbx/lstbx/normal_eqns_solving.py b/scitbx/lstbx/normal_eqns_solving.py index 660304eab..45f070622 100644 --- a/scitbx/lstbx/normal_eqns_solving.py +++ b/scitbx/lstbx/normal_eqns_solving.py @@ -140,15 +140,20 @@ class iterations(object): a = self.non_linear_ls.normal_matrix_packed_u() a.matrix_packed_u_diagonal_add_in_place(value*a. matrix_packed_u_diagonal())
- def do_scale_shifts(self, max_shift_over_esd): + def do_scale_shifts(self, limit_shift_over_su): x = self.non_linear_ls.step() esd = self.non_linear_ls.covariance_matrix().matrix_packed_u_ diagonal() - x_over_esd = flex.abs(x/flex.sqrt(esd)) - max_val = flex.max(x_over_esd) - if max_val < self.convergence_as_shift_over_esd: + ls_shifts_over_su = flex.abs(x/flex.sqrt(esd)) + #max shift for the LS + self.max_ls_shift_over_su = flex.max(ls_shifts_over_su) + jac_tr = self.non_linear_ls.actual.\ + reparametrisation.jacobian_transpose_matching_grad_fc() + self.shifts_over_su = jac_tr.transpose() * ls_shifts_over_su + self.max_shift_over_su = flex.max(self.shifts_over_su) + if self.max_shift_over_su < self.convergence_as_shift_over_esd: return True - if max_val > max_shift_over_esd: - shift_scale = max_shift_over_esd/max_val + if self.max_ls_shift_over_su > limit_shift_over_su: + shift_scale = limit_shift_over_su/self.max_ls_shift_over_su x *= shift_scale return False
------------------------------
To unsubscribe from the CCTBX-COMMIT list, click the following link: https://www.jiscmail.ac.uk/cgi-bin/webadmin?SUBED1=CCTBX-COMMIT&A=1
Sorry Nick,
please let me know - I can pull it back or tweak as needed...
Cheers,
Oleg.
________________________________
From: [email protected]
OK, I've had a chance to run the tests and everything seems to check out.
For XFEL we use the "levenberg_marquardt_iterations_encapsulated_eqns" that
seems to be not affected by your code commit.
Nick
Nicholas K. Sauter, Ph. D.
Senior Scientist, Molecular Biophysics & Integrated Bioimaging Division
Lawrence Berkeley National Laboratory
1 Cyclotron Rd., Bldg. 33R0345
Berkeley, CA 94720
(510) 486-5713
On Wed, Sep 5, 2018 at 8:42 AM, [email protected]
Sorry Nick,
please let me know - I can pull it back or tweak as needed...
Cheers,
Oleg. ------------------------------ *From:* [email protected]
on behalf of Nicholas Sauter *Sent:* 05 September 2018 16:37:53 *To:* cctbx mailing list *Cc:* [email protected] *Subject:* Re: [cctbxbb] [git/cctbx] master: re-defining the shift/est convergence limit in terms of crystallographic rather than independent parameters to make it easier to relate to the convergence reported in the CIF; also caching the shifts for further analys... I'm worried about making changes like this to a fundamental toolbox component like normal equations solving. We use this for processing xfel data, and would like the opportunity to test any changes through a pull request, prior to having them committed to the main branch.
Nick Sauter
Nicholas K. Sauter, Ph. D. Senior Scientist, Molecular Biophysics & Integrated Bioimaging Division Lawrence Berkeley National Laboratory 1 Cyclotron Rd., Bldg. 33R0345 Berkeley, CA 94720 (510) 486-5713
On Wed, Sep 5, 2018 at 5:29 AM, CCTBX commit
wrote: Repository : ssh://g18-sc-serv-04.diamond.ac.uk/cctbx On branch : master
------------------------------
commit 9339fd5f7d240f0d6f85dd57ce42e796d19608f1 Author: pcxod
Date: Wed Sep 5 13:29:29 2018 +0100 re-defining the shift/est convergence limit in terms of crystallographic rather than independent parameters to make it easier to relate to the convergence reported in the CIF; also caching the shifts for further analysis and reporting;
------------------------------
9339fd5f7d240f0d6f85dd57ce42e796d19608f1 scitbx/lstbx/normal_eqns_solving.py | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/scitbx/lstbx/normal_eqns_solving.py b/scitbx/lstbx/normal_eqns_solving.py index 660304eab..45f070622 100644 --- a/scitbx/lstbx/normal_eqns_solving.py +++ b/scitbx/lstbx/normal_eqns_solving.py @@ -140,15 +140,20 @@ class iterations(object): a = self.non_linear_ls.normal_matrix_packed_u() a.matrix_packed_u_diagonal_add_in_place(value*a.matrix_ packed_u_diagonal())
- def do_scale_shifts(self, max_shift_over_esd): + def do_scale_shifts(self, limit_shift_over_su): x = self.non_linear_ls.step() esd = self.non_linear_ls.covariance_matrix().matrix_packed_u_diago nal() - x_over_esd = flex.abs(x/flex.sqrt(esd)) - max_val = flex.max(x_over_esd) - if max_val < self.convergence_as_shift_over_esd: + ls_shifts_over_su = flex.abs(x/flex.sqrt(esd)) + #max shift for the LS + self.max_ls_shift_over_su = flex.max(ls_shifts_over_su) + jac_tr = self.non_linear_ls.actual.\ + reparametrisation.jacobian_transpose_matching_grad_fc() + self.shifts_over_su = jac_tr.transpose() * ls_shifts_over_su + self.max_shift_over_su = flex.max(self.shifts_over_su) + if self.max_shift_over_su < self.convergence_as_shift_over_esd: return True - if max_val > max_shift_over_esd: - shift_scale = max_shift_over_esd/max_val + if self.max_ls_shift_over_su > limit_shift_over_su: + shift_scale = limit_shift_over_su/self.max_ls_shift_over_su x *= shift_scale return False
------------------------------
To unsubscribe from the CCTBX-COMMIT list, click the following link: https://www.jiscmail.ac.uk/cgi-bin/webadmin?SUBED1=CCTBX-COMMIT&A=1
_______________________________________________ cctbxbb mailing list [email protected] http://phenix-online.org/mailman/listinfo/cctbxbb
OK, thanks Nick - it is actually good to know as we have been working on that one too... So we will synchronise it with you before committing,
Best wishes,
Oleg.
________________________________
From: [email protected]
participants (2)
-
Nicholas Sauter
-
oleg@olexsys.org