gafusion
Force 64bit encoding for MDSplus data
This change makes the MDSValue.raw() data fetching method always return 64-bit floats. This is important because the MDSplus data itself is stored as 32-bit and it retains this datatype when passed to OMAS. Since OMAS does some math with these values using 64-bit for 32-bit data reduces round-off errors. While this is (probably) irrelevant for physics problems, the round-off error can make regression tests harder. E.g. yesterday I encountered difference in ECE frequency of around 768 Hz when comparing between OMAS and the new IMAS_composer. The reason why this appears is because in the conversion from MDSplus [GHz]-> IMAS [Hz] the frequency is multiplied by 1.e9 which then makes the round-off error visible during the comparison.
@torrinba I snuck in a temporary fix for #379, specifically for the flux_loops. The issue should not be closed because of this fix but I just wanted you to be aware that the very specific problem I was facing is for now solved. There is one more fix where I changed the extrapolation values for fluxfun to use the boundary values instead of 0 since that seemed better than just dropping everything to zero because we are just very slightly off in psi.
Those seem like good changes, but another reason #379 shouldn't be closed is that when the COCOs transformations are automatically regenerated (with make cocos) it will remove the line you added. So it is likely to be unintentionally removed at some point
Yeah, I just wanted to push what was needed to get OMAS to actually pass its tests against IMAS_composer xD
Well unfortunately that's why all the tests are now failing. There is a check that all COCOs are "updated" so the manual entry is breaking things.
We would need to add a special case in https://github.com/gafusion/omas/blob/e5f6cc89d936c2ca0d19757647dd5667a5283e38/omas/omas_physics.py#L3121 if you want this to be merged
Merging this is not really high-priority, since it is only needed to run the tests of IMAS_composer. It can wait for a fix for #379. Maybe a hotfix for just flux_loop though?
I put in functions that should handle the EFIT constraints properly. I can see the COCOs transformations being applied to the flux loop uncertainties now. It's not the most elegant solution and these would probably be better off in a new _efit.py subroutine than in _common.py but it should be good enough to close #379 at least.
This is just how it should not be done. There is no real reason to save any experimental data in double precision. It only takes more memory, space, and computation time. A special case is a timebase for high-time-resolution signals, but the timebase should not be fetched at all; instead, it should be constructed from the PTDATA header information.
If the tests cannot tolerate an error at the 7th digit, then the tests should be rewritten. Any attempt to compare two floats should be done with np.allclose with a reasonable tolerance.
I understand your point, but I was just really unhappy that say 140 GHz in the ECE MDSPlus tree would turn into 140000547463 Hz in IMAS. Now in practice this does not make difference but as a human I find it confusing. Note that nobody is saving anything here, this is just how the data is stored in memory and the bit-length can be changed by whatever application uses IMAS_composer/OMAS. I am mainly worried about truncation error. OMAS/IMAS_composer is not just reading data, it also doing math... EDIT: I forgot I had already explained the whole ECE thing. The main thing I was worried about is that if we get a single precision signal from MDSplus we need to convert it to double precision for any math operation we do in IMAS_composer/OMAS to guarantee we maintain at least single precision for the mapped quantity.
@torrinba I have removed pymongo as a dependency that is installed during the regression testing. I really don't think we should keep wasting our time with this constantly failing test that is for a feature we don't use.
@torrinba I have removed pymongo as a dependency that is installed during the regression testing. I really don't think we should keep wasting our time with this constantly failing test that is for a feature we don't use.
It seems like that was overdue. I'm not familiar with how pymongo is used in omas though or if there are still any active use cases. I would encourage @smithsp to weigh in if he knows more about whether it should still be supported
mongo as a backend was introduced by @orso82 as a way to store omas objects to an online database. I would say that it didn't get much usage. I don't know if any @gafusion/omas_developers are still using it.
https://github.com/gafusion/omas/pull/330#issuecomment-2591087853 I agree it can be safely removed
Safely removed from the test dependencies or deprecated and completely removed?
"If it's not tested, it's broken."