-
Notifications
You must be signed in to change notification settings - Fork 15
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
Fill out more of the EFIT mapping to MDS+ #255
Conversation
This may not be 100% complete, but should be pretty close.
I am leaving it up to you to test this out @AreWeDreaming There several areas that need more attention still
|
Changes will be made in EFIT to make these usable
This includes nearly all of the additional EFIT variables in MDS+ that I know how to mapp to IMAS.
There were other name changes I had missed. This takes care of everything that comes out of EFIT now.
I think I got this covered in my next commit (not yet pushed because it needs to be tested) |
I also forgot that the |
After some modifications of OMAS I got most of the fields to load, but the following do not have data for my test case
|
@bechtt Could you take a look at this list and note any fields that should be set by a standard EFIT run but aren't. If none of these are expected to be set then that's fine, we'll add them on CAKE side of things. |
@orso82 Any idea why the following TDI expression would cause trouble:
with
MDS+ just returns an error with no info on what's actually wrong. |
Most of the issues from The |
I fixed the first and third. I can confirm the quantities in the second and fourth are in MDS+ so I expect the problem is with the TDI (possibly the dimensions) |
I'll retest.
|
There is no more broken fields now for @bechtt's test case. |
@orso82 If you could take a look at my changes to OMAS machine it would be much appreciated. The main change is that you can now load all available mappings by using a wildcard in the |
@AreWeDreaming do you understand why regression is failing? It passed prior to 02fef13 |
I'll have to run the regression tests locally. The error before was related to OMFIT things, now it doesn't seem to give us any information at all. The lines that were changed in the hash you pointed out don't exist anymore... |
Looks good to me I am happy to see that someone else other than me was able to generate the updated IMAS JSON data dictionary files! 👍 |
I may have merged https://github.com/gafusion/OMFIT-source/pull/6708 prematurely, since this one is not passing regressions. It was passing regression on the OMFIT side... |
If I run
Note that this does not seem to be the same problem as reported in the automatic tests. |
Thanks for helping test this @smithsp. We didn't make changes to any of the DIII-D machine mappings and I don't see any changes to the IMAS schema for From what I can tell |
Need to check |
Midway of testing |
It is related to the orientation of the flux matrix loaded into OMAS from the MDS+ machine mapping. There is an issue with the g-file to OMAS conversion being resolved in this PR https://github.com/gafusion/OMFIT-source/pull/6684 I'm surprised, but happy to hear that the orientation in MDS+ must be different |
This may not be 100% complete, but should be pretty close.