Skip to content

jacobian solver for matching - #1164

Open
swhite2401 wants to merge 4 commits into
masterfrom
jacobian_match
Open

swhite2401 wants to merge 4 commits into
masterfrom
jacobian_match

Conversation

@swhite2401

Copy link
Copy Markdown
Contributor

This PR introduce the XSuite jacobian method for matching into AT.

The jacobian is calculated using the response_matrix module from AT, several changes were required for matching application:
-global variable preventing to pickle the ring at each call is activated for fork start method
-possibility to use existing mp pool, this prevent creating a new mp pool every time the jacobian is updated
-possibility to compute the jacobian using one sided delta (facotr 2 speed-up)

The default behavior of response_matrix is preserved.

This PR was used as a benchmark for Claude assisted developments.

@swhite2401

Copy link
Copy Markdown
Contributor Author

jacobian_matching.ipynb

Here is a notebook showing the usage and advantages of the new jacobian solver.

@swhite2401

Copy link
Copy Markdown
Contributor Author

Parellization is not so efficicent for this simple test, I will try on a heavier test case

@lfarv

lfarv commented Sep 30, 2026

Copy link
Copy Markdown
Contributor

Excellent !The notebook works perfectly and the method looks efficient.

Are you sure that using a global for the variables really improves the speed ? As a consequence, you have to rebuild a VariableList several times (ex.: variables = [_globvars[i] for i in ivars], appearing 3 times), which has to be compared with the cost of pickling the active variables. Removing the global would simplify significantly. But if the gain is noticeable, that's ok.

By the way, this should rather be variables = VariableList(_globvars[i] for i in ivars) to make the type checker happy. It does not cost anything: VariableLIst has no __init__ method.

@swhite2401

Copy link
Copy Markdown
Contributor Author

Variable list is not pickled because of the persistent pool, if we pickle VariableList we would then need to send new updated variables every time the jacobian is recalculated.
This results in a gain of time only if the variable list carries a ring (20% on the notebook example), recomputing the variables is not a big overhead.

However, an alternative was proposed using the pool initializer, this helps also for other start method, we should consider it for platticepass and other methods using python multiprocessing!

@lfarv

lfarv commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

This looks interesting. For a better understanding, it could be wise to rename the resp_fork function which apparently is now used for all startup methods, to something like resp_mp or so.

@swhite2401

Copy link
Copy Markdown
Contributor Author

I have gone through a final round of simplification / optimization. Ready to take comments, meanwhile I switch back to testing integrators refactoring

Comment on lines +219 to +237

def _step(step_fun):
step_fun(ring=ring)
observables.evaluate(ring, **kwargs)
return observables.flat_values

x0 = variable.initial_value
vmin, vmax = variable.bounds
delta = variable.delta
down = x0 - delta >= vmin
up = x0 + delta <= vmax or not down
if up and down and f0 is None:
fp, fm, h = _step(variable.step_up), _step(variable.step_down), 2.0
else:
base = _step(variable.reset) if f0 is None else f0
if up:
fp, fm, h = _step(variable.step_up), base, 1.0
else:
fp, fm, h = base, _step(variable.step_down), 1.0

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I do not see any good reason why we should restrict the response computation with bounds. The goal is to compute the derivative at point x0 (which is within bounds). nothing will burn if the step is outside the bounds. The bounds are there to prevent the matching to overpass them, but this does not apply to response computation.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Nothing will burn for sure, but the bounds are set at the Variable level so it raises an error if we try to set it to a value beyond the bounds, right?
Then I agree with you.... bounds should be ignore when computing the response matrix, should I implement this?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Oh, I did not think to that…

I think that the cleanest solution is to add a force boolean keyword to the Variable.set and similar methods. This is trivial and I opened a separate PR for that (#1169). Have a look

This branch has not been deployed

No deployments
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.

2 participants