Update to b2mn parsing #23
Conversation
- Hopefully this handles a wider variety of b2mn instances
|
@sbdepascuale If you check out this branch within SOLPS2IMAS, you can test whether If you have an existing julia session open, you will have to restart julia unless you imported Revise ( |
|
Can confirm demo.ipynd workflow correctly executes all calls with KSTAR case under b2mn_parse branch of SOLPS2IMAS. Furthermore, interferometer data extraction from "ids.interferometer" functions as expected. |
|
Few more checks need to be added to conform with all possibilities of the file. I'm adding Jeremy's email instructions here for documentation.
I'll edit some sample files to incorporate all these test cases and test. I think we just need to add some more checking for special characters and also change all case to lower before adding keys. |
|
Found another bug in the current implementation. It distinguishes between Float64 and Int data types by checking if there is a '.' in the value field, but many value fields are written in '1e-10' format, so they get parsed as strings. I think, we should not use hidden error catching and instead should have designated code for all cases we can think of so that the parser fails when it is not parsing correctly. |
|
Oh, oops. All the samples I saw were like 1.0e-10. But if this file is human writable (and it is), then I guess 1e-10 could happen, too. |
|
The type will depend on the original Fortran, you cannot necessarily tell from the format of the value. Same goes for booleans and ints. |
Removed:
deleted: samples/b2mn.dat.sample_50dn
deleted: samples/b2mn.dat.sample_50xd
deleted: samples/b2mn.dat.sample_si1
deleted: samples/b2mn.dat.sample_si2
Added:
new file: samples/b2mn.dat.json.dvc
new file: samples/test_b2mn.dat.dvc
new file: samples/test_b2mn.dat.json.dvc
Adding a json file of correctly parsed version of b2mn.dat file that is
already in the samples directory. Also added a new test_b2mn.dat file
that is manually edited to cover all edge cases of the parser and a
json file version of the same that has been correctly parsed (checked
manually).
Updated parser so that it reads b2mn.dat file with following rules:
* Strip all leading and trailing white spaces (including tabs) in a line
* Ignore all lines until *enphy is found
* Characters #, *, and ! indicate comments. These can be after name
value pairs or if at the beginning of the line the whole line is skipped
* Name value pairs are matched case insensitive but exact strings.
These have at least two valid representations:
* Value and name in single quotes
* 'b2mwti_jxa' '36' # optional comment
* Name in single quotes but value is not
* 'b2mwti_jxa' 36 # optional comment
* Double quotes work here as well.
* The comment section can also contain quotation marks
* Due to exact string matching, be aware of “commented out” variables
that could look like this.
* '#b2mwti_jxa' 36 # Variable unused
* '!b2mwti_jxa' 36 # Variable unused
* If a name value pair is repeated the last one is used
* Read a value containing '.' or 'e' as a float and the rest as an
integer.
| end | ||
| # Get key and value in lowercase | ||
| key = lowercase(name_value[1]) | ||
| value = lowercase(name_value[2]) |
There was a problem hiding this comment.
Does this file have arrays in it? The python version seems to be expecting to get multiple values sometimes, but it also is supposed to work for two of the SOLPS files. Maybe b2mn doesn't have multiple values; maybe that's the other file.
There was a problem hiding this comment.
I'm not sure. @jlore didn't mention any possibility of arrays in these fields, but we can expand it if that is a possibility.
|
@jlore Currently, the code is reading anything with a decimal point or 'e' in it as float and everything else as int. Is there a possibility that this might cause an issue later. Do we have a list of all the fields that fortran keeps as float and all that fortran keeps as int (maybe indices). Or is there a nomenclature that we can use to determine this information. The julia functions are all type specific and using floats and ints interchangeably will cause errors in future. However, right now, solps2imas only uses b2mn.dat to read inner and outter midplane indices defined in 'b2mwti_jxi' and 'b2mwti_jxa'. |
|
There is no requirement in SOLPS that a REAL type be initialized using 'e' or a decimal point, so I don't think you should make that assumption here. Is Julia strictly typed in that you have to know the Fortran type? We can, of course, get the type from the Fortran source but I'm not sure why there is a need. Also, I don't think it is required to read "all" of the variables from b2mn.dat for our project. Why not just look for the small number of variables we will actually use (for which I can tell you the Fortran type) and ignore the rest. There are probably at least 50 variables that are not present in any of the b2mn.dat examples you are using, and these often change with the code base. Also, SOLPS ignores variable names that it doesn't use, which is important so that an obsolete variable definition does not cause the code to stop. Suggest that you just make a list of the variables that are actually needed for the project and we make a robust parser that works for these. I can then also provide you the default values (and/or logic for computing the defaults) for cases where they are not present. |
|
I just think it's awkward to parse number of steps (for example) as a float when it's clearly an int. Maybe we can parse all numbers as float64 by default and keep a list of things to force to int? The python implementation was intended to preserve type more carefully because it was used to read as well as write, and writing 3.5 steps seems like it wouldn't do anything good. |
|
Agreed. Suggest specific treatment of a limited number of variables. If you have a list of the ones you use then I can confirm the original type. |
|
From the discussion, we have these two possibilities: Whitelisting solutionOur code, specifically, SOLPS2IMAS.solps2imas() function, only uses Handle types in code where values are usedSince Julia is type specific, we should write Julia code to force cast into integer or float whenever we use the read values. This is the case already in solp2imas() actually https://github.com/ProjectTorreyPines/SOLPS2IMAS.jl/blob/37b04d7ca0eaba0251f3029590152808dbee8d35/src/SOLPS2IMAS.jl#L99 Let me know what you all prefer. |
|
I can certainly provide the logic for default values of jxa and jxi. It depends on the grid topology, but it is straightforward |
|
@jlore Yes, please. It's valuable to have a way to set defaults for anything that's used as much as jxa and jxi. Beyond that, I think the logic should be: List of things to force to non-float if possible: List of things to populate using default values if they're missing: |
Created a new file in src/b2mn_int_fields.txt where names of all known int fields are listed. This file is used to check if a field value should be converted to integer. Also, if there is a second value listed in this file in a row, that field is considered to be required and a default value is used if it is not found in the input file. Updated sample json files with additional int fields that get added by default.
|
I used the useful comments in b2mn.dat present in |
|
Ah good, I'm glad my comments were useful! |
|
I realized that since @eldond opened this PR originally, and I pushed commits after that, I can't set him as a reviewer for this PR. Thus, I've set @dautt-silva as a reviewer. Please complete the review and mark approve if it looks good. |
|
Good work. |
|
Here is some logic for default values of jxa, jxi. First you need to identify the topology. Use nncut and nnreg from b2fgmtry Then you can guess jxa, jxi. These guesses can be pretty far off depending on the poloidal spacing, but these are the code defaults. Use leftcut and rightcut from b2fgmtry. if strcmp(Geo.geometry,'SN') |
|
Since this change to b2mn parsing was already merged, I opened #26 to keep track of the proposed method for guessing jxa and jxi. |
OMFIT-source/omfit/omfit_classes/omfit_solps.pythat parses b2mn or b2ag.