search

geoschem / integrated_methane_inversion

Integrated Methane Inversion workflow repository.
https://imi.readthedocs.org
MIT License
26 stars 22 forks source link

IMI preview does not output errors to imi_output.log #136

Closed nicholasbalasus closed 4 months ago

nicholasbalasus commented 1 year ago

Name and Institution (Required)

Name: Nick Balasus Institution: Harvard University

Description of your issue or question

This is low priority. Working from the dev branch, I find that errors in the IMI preview (specifically from imi_preview.py) are not output to imi_output.log as intended (code that should address this below).

https://github.com/geoschem/integrated_methane_inversion/blob/9d6a45a1eaf355c00e6bd63686957317905fcca8/src/inversion_scripts/imi_preview.py#L99-L101

For example, I add a divide by zero error here:

https://github.com/geoschem/integrated_methane_inversion/blob/9d6a45a1eaf355c00e6bd63686957317905fcca8/src/inversion_scripts/imi_preview.py#L60

The preview will fail. However, the divide by zero error is nowhere to be found (it is not in the slurm-*.out files in the PreviewDir). The only indication in imi_output.log is that something has a gone wrong (snippet below), but we can't see what.

(standard_in) 1: syntax error
src/components/preview_component/preview.sh: line 114: [: : integer expression expected
Statistics (setup):
Preview runtime (s): 88
Note: this is part of the Setup runtime reported by run_imi.sh

This is a general problem for all of our scripts called with sbatch, as they will typically print the error to a slurm file. The reason imi_preview is not sending them even to the slurm file is (I think) related to the above error handling code being inside a function (and not the function where I put the error-causing code). Happy to put this in a future PR if anyone has a recommended solution.

sabourbaray commented 1 year ago

Yes we're on PBSPro and I also notice I have to go through different files for error handling sometimes. Also it might be the case that logs are named something other than "imi_output.log" if running multiple inversions, so the name being hardcoded can cause some confusion.

github-actions[bot] commented 5 months ago

This issue has been automatically marked as stale because it has not had recent activity. If there are no updates within 7 days it will be closed. You can add the "never stale" tag to prevent the issue from closing this issue.

msulprizio commented 5 months ago

@nicholasbalasus @sabourbaray Is this still an issue or have we resolved it? I have noticed sometimes you need to look in the stderr output (e.g. the slurm output file on our system) to find error messages.

nicholasbalasus commented 5 months ago

Hi Melissa - I think this is still an issue. I'll take a shot at fixing it.

nicholasbalasus commented 4 months ago

Closed by #199