E3SM-Project / polaris

Testing and analysis for OMEGA, MPAS-Ocean, MALI and MPAS-Seaice
BSD 3-Clause "New" or "Revised" License
6 stars 13 forks source link

`inputs` attribute of Step is not being updated #91

Closed altheaden closed 1 year ago

altheaden commented 1 year ago

The attribute self.inputs in the Step class is never touched beyond initialization, and therefore the step has an empty list of input files. When I wrap a for loop over the input files in a step in polaris.run.serial._run_step():

missing_files = list()
print('DEBUG: listing all input files in step.inputs:')
for input_file in step.inputs:
    print(f'DEBUG: {input_file}')
    if not os.path.exists(input_file):
        missing_files.append(input_file)
print('DEBUG: finished listing all input files in step.inputs.')

I get these results:

  * step: analysis
DEBUG: listing all input files in step.inputs:
DEBUG: finished listing all input files in step.inputs.

(despite the analysis step requiring the outputs of all previous steps as its inputs).

This causes missed files to go unnoticed, resulting in unhandled exceptions that we intended to capture before the step begins executing.

xylar commented 1 year ago

@altheaden, thanks very much for posting this issue!

It looks like we have these lines in compass: https://github.com/MPAS-Dev/compass/blob/8440b418533f1e1a3e78cef9cc03f4535f1b6b93/compass/step.py#L780-L781

And it seems like the equivalent is missing in polaris: https://github.com/E3SM-Project/polaris/blob/0ce1674dd4c8350c16fb366df6a0584446f6e052/polaris/step.py#L500-L507

I believe what happened is that converting the inputs to absolute paths got moved to a helper function here (partly to make the linter happy): https://github.com/E3SM-Project/polaris/blob/0ce1674dd4c8350c16fb366df6a0584446f6e052/polaris/step.py#L587 But then we are missing the final line here following the for loop defining inputs:

        inputs = []
        databases_with_downloads = set()
        for entry in self.input_data:
            input_file, database_subdirs = Step._process_input(
                entry, config, base_work_dir, component, step_dir)
            if database_subdirs is not None:
                databases_with_downloads.update(database_subdirs)
            inputs.append(input_file)
        self.inputs = inputs

Could you see if that 1-line fix changes what you see in polaris.run.serial? You will need to set up the test cases or suite again to see the change (i.e. the pickle files will need to be modified to have the missing inputs attributes for each step.)

altheaden commented 1 year ago

@xylar This is the fix I tried last night, but I was getting some weird output in one of the log files. Let me make a fresh run right now and see if I'm still seeing that behavior, and if so, I'll post it here for you to look at.

xylar commented 1 year ago

The output you pointed out to me on Slack looked normal to me. That output is what comparison with the baseline looks like -- a variable and some time levels where they get compares. (Maybe that's not what you're talking about, though.)

altheaden commented 1 year ago

Oh, I see! That makes sense then, I was comparing it to the output from the actual baseline run. In that case, it seems to be working fine, all the baseline checks ran as expected. I'll go ahead and put together a PR for the branch I've got.

xylar commented 1 year ago

Sounds good!