I conducted some performance tests to see if missing_value versus _FillValue had any impact on write times. I used the MPAS-Ocean core with each, and the changes in #571. Writing 1GB on 4 nodes, I get identical times between missing_value versus _FillValue. Look across yellow cells, which is PIO2. This shows the mean and std dev for 10 tests.
Numbers are write time in seconds:

By both (right columns) I mean I added both lines:

src/tools/registry/gen_inc.c
1407 fortprintf(fd, "      call mpas_add_att(%s %% attLists(const_index) %% attList, 'missing_value', %s)\n", pointer_name_arr, missing_value);
1408 fortprintf(fd, "      call mpas_add_att(%s %% attLists(const_index) %% attList, '_FillValue', %s)\n", pointer_name_arr, missing_value);
1635 fortprintf(fd, "      call mpas_add_att(%s %% attLists(1) %% attList, 'missing_value', %s)\n", pointer_name_arr, missing_value);
1636 fortprintf(fd, "      call mpas_add_att(%s %% attLists(1) %% attList, '_FillValue', %s)\n", pointer_name_arr, missing_value);

The main message is that PIO2 is much faster than PIO1! But for this PR, the attribute name makes no difference with PIO2, but does make a slight difference with PIO1.

@mark-petersen, thank you for doing this timing. Based on the results, I would advocate for just using _FillValue and not missing_value, since both seems like a bad option for PIO1.

I will do some testing of my own shortly and hopefully approve the PR.

@mgduda and @MiCurry, are you and your collaborators using both PIO1 and PIO2 on a regular basis?

@xylar I'm using PIO2 on almost every machine where I regularly build MPAS. But, I suspect there are a number of our collaborators who use PIO1 (if only because it can be easier to install). I don't think we should break compatibility with PIO1, though I don't see any issues in that regard with this PR. I agree that we should use just the _FillValue attribute, though, and not both _FillValue and missing_value.

@mark-petersen In the description of this PR, would it be more accurate to say that this merge "changes the name of the missing value attribute from missing_value to _FillValue" rather saying that this merge "sets missing value attribute to _FillValue"? I think the former would make it clearer that there already was a missing value attribute, and that we're changing its name.

It might also be helpful to summarize why we're making this change. Given the amount of time we all have spent in arriving at to this PR, I think a few sentences are in order. I can even add these if you're busy this week.

While these changes do add a _FIllValue attribute to a field, it, unfortunately, undoes the changes from PR #562. i.e. it doesn't fill the field with missing_value. Perhaps I am wrong, but wasn't the intent of these changes to have 'missing_value' in the registry add a _FillValue attribute to a field and fill the field with missing_value?

Checking out 6525f33 (the commit before), and running the same test the field is filled with 'missing_value'.

@MiCurry, could you explain a bit more how you tested? Looking at the code, I don't understand why #562 would be affected. We are only changing the name of the attribute in the Fortran data structure, not the name in the registry. My reading of #562 is that it gets missing value from parsing the XML from the registry so it should still have missing_value as before. But I have not tested this yet myself.

Following up, I reran my test from before in which I created a variable that is all zeros about the MPAS-ocean bathymetry an fill values (filled using @MiCurry's changes in #562) below it. I created a variable array that is all ones above the bathymetry and all fill values below. That worked out just fine for me. So I don't think this PR is having any negative effect on the changes in #562. @MiCurry, can you please provide steps to reproduce the problem you're seeing?

Sorry about the delay. @xylar, I was trying to see if I could get a little more information on what I was experiencing. I'm quite confused myself as to what is happening. I created a nCells dimensioned integer field with missing_value="-1":

<var name="int_field" type="integer" dimensions="nCells" missing_value="-1"/>

And ran the init_atmosphere_model to interpolate real-data conditions and did not alter the code, so int_field is not being accessed or modified. After running the field correctly has the _FillValue attribute:

int int_field(nCells) ;
     int_field:_FillValue = -1 ;

But the field was not set to -1 and had '-'.

Switching the field to type=real though did set the array to -1.

<var name="real_field" type="real" dimensions="nCells" missing_value="-1"/>

But then it doesn't set the _FillValue attribute for real_field.

Perhaps I am setting things up incorrectly. I'll try another test on a different machine and see how that goes.

@MiCurry, the first example with int_field sounds like the expected behavior to me. ncdump will display - instead of the numerical value for values equal to the _FillValue attribute. So your array is presumably full of -1, but you won't see it.

The second case sounds odd. The _FillValue should have propagated through fine for a real just like for an int.

@MiCurry, the first example with int_field sounds like the expected behavior to me. ncdump will display - instead of the numerical value for values equal to the _FillValue attribute. So your array is presumably full of -1, but you won't see it.

Ah that makes sense! My mistake!

The second case sounds odd. The _FillValue should have propagated through fine for a real just like for an int.

I think I've discovered it. Looks like missing_value="-1" does not work with floats/reals, instead it needs to be: missing_value="-1.0". Switching it to missing_value="-1.0" propagated _FillValue down to the field.

Hmm, maybe we should fix that. Seems like it will happen to others, too.

@mark-petersen In the description of this PR, would it be more accurate to say that this merge "changes the name of the missing value attribute from missing_value to _FillValue" rather saying that this merge "sets missing value attribute to _FillValue"? I think the former would make it clearer that there already was a missing value attribute, and that we're changing its name.

It might also be helpful to summarize why we're making this change. Given the amount of time we all have spent in arriving at to this PR, I think a few sentences are in order. I can even add these if you're busy this week.

@mark-petersen everything looks good to me, but I agree with @mgduda on the above. I also think it would be nice to be more explicit in the PR message e.g. 'The _Fillvalue attribute can be set by using missing_value in the registry'. I think that would be a clearer example than PR #571.

@mgduda and @MiCurry, is the new description adequate for your needs?

I just added the words "in netCDF output files" to distinguish between XML attributes in the Registry and netCDF attributes. I also wrapped the description so that in plain text the lines won't be longer than 80 columns.