Bugfixes to labelling #569
Conversation
matt-frey
added
bug
src/3d/parcels/parcel_netcdf.f90
Outdated
| @@ -666,14 +666,16 @@ subroutine read_chunk(first, last, pfirst) | |||
| #ifdef ENABLE_LABELS | |||
| if (has_dataset(ncid, 'label')) then | |||
There was a problem hiding this comment.
Note that you can just write `l_valid = (l_valid .and. has_dataset(ncid, 'label')). However, you initialise default values. Therefore, this check is not necessary at all.
There was a problem hiding this comment.
It is a bit confusing. We do need the old values to be present for successful tracking, even though we overwrite them immediately after we write to file. One advantage of the current syntax is that it is the same as other variables, which makes it easy to read. I suggest adding a note that "existing" labels are needed for tracking. What do you think?
There was a problem hiding this comment.
Not sure if I can think of circumstances where we have just labels and/or dilution but thought there might be a future or test case use.
There was a problem hiding this comment.
What do you mean with "We do need the old values to be present for successful tracking, even though we overwrite them immediately after we write to file."? Here, you read parcel attributes. You do not write them. This is used in restarts.
There was a problem hiding this comment.
What we so in a normal (non-restart) run is reinitialise the labels after writing them to disk. The reinitialisation (in the lines below) is important for backward tracking so that we can uniquely identify the "parent" parcel in the previous time-step (and then go backward by iterating this).
However, this iterative tracking relies on dumping the labels in every parcel file. If you want tracking to behave in the same way in a restart as in an uninterrupted run (at least statistically, because of the use of random numbers in merging which is currently not reproducible though we could work on this), you need to ensure that a "label dump" is present in the restart file.
There was a problem hiding this comment.
Note that we call parcel_default before entering this routine. You can set the labels and dilution there and then do not read in labels and dilution in the read routine at all.
There was a problem hiding this comment.
Yes, y_vorticity misses the logical check l_valid.
There was a problem hiding this comment.
Sure, I can remove the l_valid checks for now. We can always put it back when needed. Do you mind if I leave the rest as is and get on to other things?
There was a problem hiding this comment.
Thanks! It should be ready now.
|
Do we still need this branch? |
Resets the labels on restarts, and fixes a wrong ifdef.