search

gafusion / omas

Ordered Multidimensional Array Structure
http://gafusion.github.io/omas
MIT License
30 stars 14 forks source link

COCOs is not applied to variable uncertainties #250

Closed torrinba closed 1 year ago

torrinba commented 1 year ago

From what I have seen, the COCOs transformations only get applied to variables specified in omas_cocos.py and not the uncertainty of those variables. Do the uncertainties need to be specified explicitly for every quantity in omas_cocos.py to be included or can this be handled more broadly?

For example, I am looking at equilibrium.time_slice.:.constraints.flux_loop.:.measured and equilibrium.time_slice.:.constraints.flux_loop.:.measured_error_upper

orso82 commented 1 year ago

Indeed we should apply the COCOS transform to the ..._error_upper quantities automatically.

@bechtt I think the few lines to update are the __getitem__ and __setitem__ functions. Specifically these lines (and abouts) https://github.com/gafusion/omas/blob/82dd96e79757b932d85913691c579e0f02a04629/omas/omas_core.py#L934 https://github.com/gafusion/omas/blob/82dd96e79757b932d85913691c579e0f02a04629/omas/omas_core.py#L1337 should change from

ulocation in omas_physics.cocos_signals

to

re.sub(r'_error_upper$', '', ulocation) in omas_physics.cocos_signals

Can you please give it a try and test it with the data you have at hand?

torrinba commented 1 year ago

Thanks for the suggestion! I had to the make substitution for all occurrences of ulocation inside the if blocks, but after that the fix worked great. I just opened an MR with the changes in #251

Is there any reason this shouldn't be applied to the ..._error_lower quantities as well? (I don't use them but could imagine someone else running into this in the future)