Skip to content

Conversation

@weiyuan-jiang
Copy link
Contributor

The EASEvxx_tile2pfaf.nc4 file is placed in the folder with the nc4 tile file. It should be excluded after 'glob' to find the nc4 tile file

@gmao-rreichle
Copy link
Collaborator

@weiyuan-jiang : Thanks for adding this bug fix. One question: Do we also need the same protection here (for 'RESTART'==2)?

in_tilefiles_ = glob.glob(inpdir+'/*.nc4')

It's a bit unclear to me why we handle the TILING_FILE for 'RESTART'==1 in a different place in the code from where we handle the TILING_FILE for 'RESTART'==2. In both cases, we have restart files (and thus a tiling file) from an existing GEOSldas experiment. Shouldn't the logic for obtaining the TILING_FILE (from the ./rc_out directory) be the same (or nearly the same) in both cases? (For 'RESTART'==2, I think we perface the tiling file name with 'MAPL_', but that could be done with an if-condition that applies to just the file name within the larger logic of assembling the path and deciding if we use the txt or nc4 file.

Also, I wonder if the code involved in the bug fix here also is related to the bug that Devon reported. It may make sense to fix both bugs in this PR (if Devon's problem is in fact a bug in GEOSldas setup).

Removed redundant glob call for nc4_tile.
@weiyuan-jiang weiyuan-jiang marked this pull request as ready for review February 11, 2026 19:12
@weiyuan-jiang weiyuan-jiang requested a review from a team as a code owner February 11, 2026 19:12
@weiyuan-jiang
Copy link
Contributor Author

weiyuan-jiang commented Feb 11, 2026

@weiyuan-jiang : Thanks for adding this bug fix. One question: Do we also need the same protection here (for 'RESTART'==2)?

in_tilefiles_ = glob.glob(inpdir+'/*.nc4')

It's a bit unclear to me why we handle the TILING_FILE for 'RESTART'==1 in a different place in the code from where we handle the TILING_FILE for 'RESTART'==2. In both cases, we have restart files (and thus a tiling file) from an existing GEOSldas experiment. Shouldn't the logic for obtaining the TILING_FILE (from the ./rc_out directory) be the same (or nearly the same) in both cases? (For 'RESTART'==2, I think we perface the tiling file name with 'MAPL_', but that could be done with an if-condition that applies to just the file name within the larger logic of assembling the path and deciding if we use the txt or nc4 file.

Also, I wonder if the code involved in the bug fix here also is related to the bug that Devon reported. It may make sense to fix both bugs in this PR (if Devon's problem is in fact a bug in GEOSldas setup).

This is not a bug for Devon because she did not setup run on EASE grid. I will make changes on the route PR when we need the tile2pfaf file in the rc_out directory. But I agree it is quite messy here. I will do some cleanup in the other PR

@gmao-rreichle
Copy link
Collaborator

This is not a bug for Devon because she did not setup run on EASE grid. I will make changes on the route PR when we need the tile2pfaf file in the rc_out directory. But I agree it is quite messy here. I will do some cleanup in the other PR

My comment wasn't meant to be specific to the EASE grid. I think the code in question executes independent of the grid/tile space. Is there a reason for the separation of what seems to be (nearly) the same action into a block for RESTART==1 and another block elsewhere for RESTART==2?

Re. Devon, I was just wondering if the TILING file is processed correctly. The bug fix here touches the lines that are dealing with the nc4 tiling file, and we suspected that Devon's problem may have to do with a mismatched TILING file.

In any case, the bug fix that the PR intends to fix is for EASE and touches LDAS setup. My suggestion was to add the bug fix for Devon (if there is indeed a bug) into this PR -- assuming the fix also touches LDAS setup. (We can rename the PR to something like "LDAS setup bug fixes...")

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants