Ops incorporation

[Wesley-J-Davis]

Small modifications to SMOSproc_main.py, which cause any failure in the preprocessing, or reg2fit steps to return a non-zero exit code that can be detected by the perl driver that we in ops use to run these scripts and return their outputs to listing files.

[biljanaorescanin]

Just to confirm the intended behavior here: I agree that we want actual preprocessing failures to propagate a non-zero exit status so the ops job does not appear successful.

There are two cases where this changes the previous behavior from continuing to aborting the entire job:

1. If no SMOS input zip files are found for one date, develop currently catches the error, advances current_date, and continues with the next day. With sys.exit(1), the whole multi day job will now stop. @gmao-rreichle, is that the desired behavior, or should a missing SMOS input product for one day remain a recoverable case where we skip that date and continue processing?

Current develop behavior:

GEOSldas_GridComp/GEOSldas_App/util/inputs/obs_preproc/SMOS_preproc/SMOSproc_main.py

Lines 114 to 120 in 6403551

	# Step 1: Convert files to NetCDF
	try:
	ncflist = process_ee_to_nc(current_date)
	except RuntimeError as e:
	logging.error(f"Halting processing for {date_str} due to error: {e}")
	current_date += timedelta(days=1)
	continue

Also, the current log message says Halting processing for ..., although the code actually skips that date and continues to the next one, so that message may be worth clarifying as well.

2. If preprocess_nc fails for one granule/file, develop currently skips that file and continues processing the remaining granules. Now one bad granule will abort the whole job. Is that also intentional, or should an individual bad granule remain a skippable error?

Current develop behavior:

GEOSldas_GridComp/GEOSldas_App/util/inputs/obs_preproc/SMOS_preproc/SMOSproc_main.py

Lines 122 to 129 in 6403551

	# Step 2: Preprocess NetCDF files into REG
	for fnc in ncflist:
	logging.info(f"Preprocessing NC file: {fnc}")
	try:
	run_in_isolated_process(preprocess_nc, fnc, config)
	except RuntimeError as e:
	logging.error(f"Skipping {fnc} due to error: {e}")
	continue

If both cases are supposed to make the ops job fail, then this change looks good to me as well.

[gmao-qliu]

There are two cases where this changes the previous behavior from continuing to aborting the entire job:

1. If no SMOS input zip files are found for one date, develop currently catches the error, advances current_date, and continues with the next day. With sys.exit(1), the whole multi day job will now stop. @gmao-rreichle, is that the desired behavior, or should a missing SMOS input product for one day remain a recoverable case where we skip that date and continue processing?

The original code intended to skip the date and continue processing. I'm not sure what the preferred behavior is in the OPS setting.

2. If preprocess_nc fails for one granule/file, develop currently skips that file and continues processing the remaining granules. Now one bad granule will abort the whole job. Is that also intentional, or should an individual bad granule remain a skippable error?

Current develop behavior:

GEOSldas_GridComp/GEOSldas_App/util/inputs/obs_preproc/SMOS_preproc/SMOSproc_main.py

Lines 122 to 129 in 6403551

	# Step 2: Preprocess NetCDF files into REG
	for fnc in ncflist:
	logging.info(f"Preprocessing NC file: {fnc}")
	try:
	run_in_isolated_process(preprocess_nc, fnc, config)
	except RuntimeError as e:
	logging.error(f"Skipping {fnc} due to error: {e}")
	continue

I think the preferable setting here would be to skip the bad file and continue.

[Wesley-J-Davis]

There are two cases where this changes the previous behavior from continuing to aborting the entire job:

1. If no SMOS input zip files are found for one date, develop currently catches the error, advances current_date, and continues with the next day. With sys.exit(1), the whole multi day job will now stop. @gmao-rreichle, is that the desired behavior, or should a missing SMOS input product for one day remain a recoverable case where we skip that date and continue processing?

The original code intended to skip the date and continue processing. I'm not sure what the preferred behavior is in the OPS setting.

2. If preprocess_nc fails for one granule/file, develop currently skips that file and continues processing the remaining granules. Now one bad granule will abort the whole job. Is that also intentional, or should an individual bad granule remain a skippable error?

Current develop behavior:

GEOSldas_GridComp/GEOSldas_App/util/inputs/obs_preproc/SMOS_preproc/SMOSproc_main.py

Lines 122 to 129 in 6403551

	# Step 2: Preprocess NetCDF files into REG
	for fnc in ncflist:
	logging.info(f"Preprocessing NC file: {fnc}")
	try:
	run_in_isolated_process(preprocess_nc, fnc, config)
	except RuntimeError as e:
	logging.error(f"Skipping {fnc} due to error: {e}")
	continue

I think the preferable setting here would be to skip the bad file and continue.

I will work on a solution to that effect. This is a good catch, we in ops also need to reserve the ability to force processing even if there'a bad granule.

I think we would prefer the default behavior in case of bad processing of a granule to fail the entire job, but yes we do need to be able to force it through.

[Wesley-J-Davis]

I am having second thoughts on these commits. I think I would be cleaner to have an operational version called SMOSproc_main_ops.py with the exit codes baked in there for the regular everyday run, to know when it fails.

We would then keep the default version of SMOSproc_main.py available for when we'd like to force-complete the day, which is the default behavior.

Otherwise it will require amending the get_time_range function in src.helper.util since the package is designed to only expect --date or --start (--end optional) or nothing at all.

What I would be doing is just taking the proposed changes to SMOSproc_main.py and applying them to SMOSproc_main_ops.py and switching between the two in the perl driver that we will be using to connect D_BOSS to the submission of this job, which is not a part of this repository.

How does everyone feel about this?

[biljanaorescanin]

Others may have other ideas, but here is my 2c.

My only concern would be duplicating the full SMOSproc_main.py into SMOSproc_main_ops.py, since then we would have two copies of essentially the same processing code to keep synchronized.

Could we instead keep SMOSproc_main.py as the single implementation and make the error behavior a parameter to main(), something like main(fail_fast=False)? Then SMOSproc_main_ops.py could just be a small wrapper that calls the same code with fail_fast=True.

That would also avoid having to modify get_time_range() or add another command-line option. The Perl driver could simply call the ops wrapper, while both entry points would continue using the existing --date / --start / --end handling.

[Wesley-J-Davis]

Others may have other ideas, but here is my 2c.

My only concern would be duplicating the full SMOSproc_main.py into SMOSproc_main_ops.py, since then we would have two copies of essentially the same processing code to keep synchronized.

Could we instead keep SMOSproc_main.py as the single implementation and make the error behavior a parameter to main(), something like main(fail_fast=False)? Then SMOSproc_main_ops.py could just be a small wrapper that calls the same code with fail_fast=True.

That would also avoid having to modify get_time_range() or add another command-line option. The Perl driver could simply call the ops wrapper, while both entry points would continue using the existing --date / --start / --end handling.

This is a good idea, thank you.

Here's my plan to incorporate it, slightly different than suggested:

In the perl driver I set an environment variable that reflects whether we want to force the job through or not:

# we need an if / else block dependent upon the force flag
if ( ! defined( $opt_F ) ) {
  $cmd = "bash -l -c 'source modules.rc && export FAIL_FAST=True && python SMOSproc_main.py --date $process_date'";
}
else {
  $cmd = "bash -l -c 'source modules.rc && export FAIL_FAST=False && python SMOSproc_main.py --date $process_date'";
}
$rc=system($cmd);

In SMOSproc_main.py I read the environment variable to determine which behavior path to choose upon failure:

# ---------------------------------------------------------
# Configuration
# ---------------------------------------------------------
CONFIG_PATH = os.path.join(os.path.dirname(os.path.abspath(__file__)),'config.yaml')
with open(CONFIG_PATH) as f:
    config = yaml.safe_load(f)['paths']

EE_TO_NC_SCRIPT           = config['ee_to_nc_script']
SMOS_BASE_PATH            = config['smos_base_path']
TMP_NC_PATH               = config['tmp_nc_path']
OUT_REG_PATH              = config['out_reg_path']

try:
    env_fail_fast = os.environ.get('FAIL_FAST').strip().lower()
    fail_fast = env_fail_fast not in ['false']
except:
    logging.info(f"fail_fast is not set. Executing code with default behavior.")
    fail_fast = False

Then later on down in main() I put the if-else blocks in to either hard exit or continue. This makes changes to the repository minimal, doesn't add new files, and allows ops to change the behavior of the code dependent upon our DBOSS configs.

How does everyone feel about this method?

[biljanaorescanin]

Yes, I like this approach. It keeps a single implementation, avoids adding another entry point, and lets the Perl/D_BOSS driver select the error behavior for each run without changing get_time_range().

The only thing I might change is the environment variable parsing so an unexpected value doesn't automatically become True. For example, we could explicitly accept true/false and either default to False when it is unset or raise an error for any other value.
So from your suggested line:
fail_fast = env_fail_fast not in ['false']
to this:
fail_fast = env_fail_fast == 'true'

Otherwise this looks good to me.

[Wesley-J-Davis]

I used a try/except block to check for the value of the FAIL_FAST environment variable. If it is set incorrectly, or unset, it defaults to the original behavior of the code.

Testing has revealed that these settings do indeed control the flow of the code as intended.

I added one logging command to run_in_isolated_process so that it's initiation shows up in the logs, which helped me follow what was happening.

[biljanaorescanin]

It looks like the main while current_date < end_time: block was accidentally duplicated after the first loop.
I think the second copy can just be removed.

One other small thing: for the process_ee_to_nc() error, the log still says Halting processing before checking fail_fast. When fail_fast=False, we actually skip that date and continue, so maybe that message should be changed to something neutral or moved inside the if/else. Like: Error building the nc file list for {date_str} ...

[gmao-qliu]

Do we have a way to be alerted if there is no input data for a given date? We normally expect 20+ zip files per day.

[biljanaorescanin]

Do we have a way to be alerted if there is no input data for a given date? We normally expect 20+ zip files per day.

Yes. In process_ee_to_nc, if there are 0 zip files for a given date, it raises a runtime error. With FAIL_FAST=True, that should propagate as a non zero exit.
But It doesn't currently flag a low file count, such as fewer than 20 files... So if we always expect 20+ files per day and want to catch incomplete days as well, that would need a separate check. Do we need that?

[gmao-qliu]

Do we have a way to be alerted if there is no input data for a given date? We normally expect 20+ zip files per day.

Yes. In process_ee_to_nc, if there are 0 zip files for a given date, it raises a runtime error. With FAIL_FAST=True, that should propagate as a non zero exit. But It doesn't currently flag a low file count, such as fewer than 20 files... So if we always expect 20+ files per day and want to catch incomplete days as well, that would need a separate check. Do we need that?

We pretty much always expect 20+ files. If fewer than that, we need to take a look.

[Wesley-J-Davis]

Do we have a way to be alerted if there is no input data for a given date? We normally expect 20+ zip files per day.

Yes. In process_ee_to_nc, if there are 0 zip files for a given date, it raises a runtime error. With FAIL_FAST=True, that should propagate as a non zero exit. But It doesn't currently flag a low file count, such as fewer than 20 files... So if we always expect 20+ files per day and want to catch incomplete days as well, that would need a separate check. Do we need that?

We pretty much always expect 20+ files. If fewer than that, we need to take a look.

The DBOSS configuration of this job requires a successful download of the SMOS data that is being preprocessed. That job is known as GET-SMOS-01. So, without a successful download of all three types of SMOS data that I'm downloading (BWLF1C,SCLF1C,SMUDP2), this job won't run at all.

This is at least a preliminary check, but we don't always get 20+. Yesterday we got 28 for this product and today we got 2.

I'll plug in a check before the ee to nc conversion step to look for a minimum number of files, sounds like we've settled on 20 as our target?

[gmao-qliu]

This is at least a preliminary check, but we don't always get 20+. Yesterday we got 28 for this product and today we got 2.

It looks like more files are still arriving based on the timestamps, so let's check back this afternoon to confirm today's total. Our estimate of 20 files is conservative based on past counts, but we'll adjust accordingly if upstream processing changes.

[Wesley-J-Davis]

Inserted the following change to check for minimum of 20 ee files before conversion to nc.

    eeflist = sorted(glob.glob(search_pattern))

    if len(eeflist) < 20:
        raise RuntimeError("Found {len(eeflist)} ee files for {date_str}. Minimum 20 ee files needed. Exiting.")
    else:
        logging.info(f"[{date_str}] Found {len(eeflist)} zip files to process. Proceeding with conversion.")