Conversation
|
@andrewgiuliani can you check this new fix? |
There was a problem hiding this comment.
Pull Request Overview
This PR improves the computation of the second derivative tensor of the magnetic field by implementing a tensor transformation from cylindrical to Cartesian coordinates. The key changes include:
- Refactoring grad_grad_B_tensor_cylindrical to use an orthonormal basis.
- Replacing an intricate manual conversion in grad_grad_B_tensor_cartesian with a rotation matrix-based transformation.
- Introducing a cleaner and more modular approach for tensor coordinate transformation.
| T_cartesian = np.einsum('pia,pjb,pkc,pabc->pijk', | ||
| R, R, R, grad_grad_B_cyl) | ||
|
|
||
| return T_cartesian.transpose(1,2,3,0) # shape (nphi, 3, 3, 3) No newline at end of file |
There was a problem hiding this comment.
The final transpose call in grad_grad_B_tensor_cartesian changes the tensor shape from (nphi, 3, 3, 3) to (3, 3, 3, nphi), which does not match the documented output shape. Please adjust the transpose to ensure the returned tensor has shape (nphi, 3, 3, 3) as specified in the docstring.
| return T_cartesian.transpose(1,2,3,0) # shape (nphi, 3, 3, 3) | |
| return T_cartesian # shape (nphi, 3, 3, 3) |
| vector B=(B_R,B_phi,B_Z) at every point along the axis (hence with nphi points) | ||
| where R, phi and Z are the standard cylindrical coordinates. | ||
| ''' | ||
| # return np.transpose(self.grad_grad_B,(1,2,3,0)) |
There was a problem hiding this comment.
[nitpick] If the commented-out code is no longer needed, it would be cleaner to remove it to reduce clutter and improve maintainability.
| # return np.transpose(self.grad_grad_B,(1,2,3,0)) | |
Similar to pyQSC's pull request
landreman/pyQSC#24