Skip to content

Commit

Permalink
change d_varphi_d_phi to use formula so it will work with pyQic (whic…
Browse files Browse the repository at this point in the history
…h does not use same name)
  • Loading branch information
dpanici committed May 15, 2023
1 parent f6cf464 commit 7e765bb
Showing 1 changed file with 4 additions and 1 deletion.
5 changes: 4 additions & 1 deletion desc/objectives/nae_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,10 @@ def _calc_2nd_order_NAE_coeffs(qsc, desc_eq):
Z2sNAE = qsc.Z2s_untwisted
Z2cNAE = qsc.Z2c_untwisted

dvphi_dp = qsc.d_varphi_d_phi # needed to convert dX_d_dvarphi derivs to dX_d_dphi
# need dvarphi_dphi to convert dX_d_dvarphi derivs to dX_d_dphi
# formula from https://github.com/landreman/pyQSC/blob/main/qsc/init_axis.py#L110
dvphi_dp = qsc.nphi / (np.sum(qsc.d_l_d_phi)) * qsc.d_l_d_phi

# coefficient derivatives
X1cp = qsc.d_X1c_d_varphi * dvphi_dp
X1sp = qsc.d_X1s_d_varphi * dvphi_dp
Expand Down

2 comments on commit 7e765bb

@SebereX
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this expression for d_varphi_d_phi is not correct for a QI (or at least it appears strange). In terms of quantities that exist in both qsc and qic, it should be something like,
d_varphi_d_phi = 1 + np.matmul(qsc.d_d_phi, qsc.varphi - qsc.phi)
Should re-run the qic case. I think this is the reason why we have discrepancy in B, which disappears if plotted in `varphi'. I'll run with this modification and see what the result is.

@dpanici
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is only for the order rho squared though, so it should not affect the QI O(rho), but I think you had gotten it right

Please sign in to comment.