Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OMAS COCOs transformations do not appear to be adequate for em_coupling #299

Closed
torrinba opened this issue Mar 20, 2024 · 8 comments · May be fixed by #300
Closed

OMAS COCOs transformations do not appear to be adequate for em_coupling #299

torrinba opened this issue Mar 20, 2024 · 8 comments · May be fixed by #300

Comments

@torrinba
Copy link
Collaborator

I'm not too familiar with what options that are available for COCOs transformations in OMAS, but from what I can tell they do not appear to be capable of supporting matrices which need to have different transformations applied for each dimension.

For example, the mutual_plasma_active matrix has units of inverse toroidal current (TOR) in one dimension (corresponding to pf_active coils) and units of flux (PSI) in the other dimension (corresponding to the poloidal flux generated by the coils).

This is a pretty unusual application for COCOs since it seems like the matrix would need to have the transformation PSI/TOR (which would be the same as PSI*TOR if TOR only involves a sign change), but I don't see any way to handle this in OMAS. Is this possible?

Currently em_coupling has no COCOs defined so any use of these variables from OMAS with non-native COCOs will produce incorrect results.

The other em_coupling matrices seem like they would have the same transformation or something trivial (TOR or None)

@orso82
Copy link
Member

orso82 commented Mar 20, 2024

@torrinba I think we might have encountered a similar situation with the MSE. @jmcclena might remember.
The approach there was to do set the COCOS transformation to be None for those variables, and handle the transformation of the individual elements by hand.

@jmcclena
Copy link
Collaborator

@orso82 That's what I was thinking at first too. But it was a bit easier for MSE, as each row had a different cocos. So we were able to set it with an array of cocoss

_cocos_signals['mse.channel.:.active_spatial_resolution.:.geometric_coefficients']=[None, None, 'TOR', None, None, None, 'TOR', None, None]#[DEL?]# -1.000000 # geometric_coefficients

@orso82
Copy link
Member

orso82 commented Mar 20, 2024

@jmcclena I had forgotten about the array of COCOS!

@torrinba please take a look if you can use the same functionality (or perhaps you can extend it to work with matrixes)

@torrinba
Copy link
Collaborator Author

@orso82 the challenge with the em_coupling matrices is not that the rows or columns need to have different COCOs, it's that each dimension does. For example using the matrix A to compute b = A x, the vectors b and x have different COCOs so A needs to have a combination of those. Does OMAS support this?

@jmcclena
Copy link
Collaborator

@torrinba The logic for handling the array can be found here:

if isinstance(transform, list):

Perhaps we can add logic to split by *, and perform all transforms in the split for em_coupling?

@jmcclena
Copy link
Collaborator

jmcclena commented Mar 20, 2024

Something like 1354 turns to:

norm = 1 
for transform in transform.split('*'):
    norm *= omas_physics.cocos_transform(self.cocosio, self.cocos)[transform]

Copy link

Stale issue message

Copy link

This issue has not seen any activity in the past 60 days. It is now marked as stale and will be closed in 7 days if no further activity is registered.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Aug 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants