Minor changes from big branch grd-failure #173

areeda · 2023-10-25T20:29:07Z

This PR :

Increase concurrent jobs fom 10 to 25 and does it using condor majobs rather than parent-child DAG
Another effort to deal with stalled online jobs it limits how long an online job will "look-back"
Logging changes

With previously merged PRs we should be compatible with the latest dqsegdb2 release.

* Address issue gwpy#126. Allow pyomicron to run from a frame cache without accessing dqsegdb. Add documentation for this * Do not merge files if they overlap "metric days" * Do not cross metric day boundaries. Co-authored-by: Joseph Areeda <[email protected]>

* Do not cross metric day boundaries. * add log file arg delete empty directories when done * Tweak remove empty dir removal * Tweak remove empty dir removal again * Merge day boundary (gwpy#146) * Address issue gwpy#126. Allow pyomicron to run from a frame cache without accessing dqsegdb. Add documentation for this * Do not merge files if they overlap "metric days" * Do not cross metric day boundaries. Co-authored-by: Joseph Areeda <[email protected]> * rebase agaist last approved PR * rebase against last approved PR * rebase against last approved PR again, fix flake8 * Fix a bug in remove empty directories. Co-authored-by: Joseph Areeda <[email protected]>

* Address issue gwpy#126. Allow pyomicron to run from a frame cache without accessing dqsegdb. Add documentation for this * Do not merge files if they overlap "metric days" * Do not cross metric day boundaries. Co-authored-by: Joseph Areeda <[email protected]>

…th-gaps

* Do not cross metric day boundaries. * Merge day boundary (gwpy#146) * Address issue gwpy#126. Allow pyomicron to run from a frame cache without accessing dqsegdb. Add documentation for this * Do not merge files if they overlap "metric days" * Do not cross metric day boundaries. Co-authored-by: Joseph Areeda <[email protected]> * Check point merge (gwpy#147) * Do not cross metric day boundaries. * add log file arg delete empty directories when done * Tweak remove empty dir removal * Tweak remove empty dir removal again * Merge day boundary (gwpy#146) * Address issue gwpy#126. Allow pyomicron to run from a frame cache without accessing dqsegdb. Add documentation for this * Do not merge files if they overlap "metric days" * Do not cross metric day boundaries. Co-authored-by: Joseph Areeda <[email protected]> * rebase agaist last approved PR * rebase against last approved PR * rebase against last approved PR again, fix flake8 * Fix a bug in remove empty directories. Co-authored-by: Joseph Areeda <[email protected]> * Merge day boundary (gwpy#146) * Address issue gwpy#126. Allow pyomicron to run from a frame cache without accessing dqsegdb. Add documentation for this * Do not merge files if they overlap "metric days" * Do not cross metric day boundaries. Co-authored-by: Joseph Areeda <[email protected]> * minor doc changes * Fix a bug where an xml.gz file could get compressed again in merge-with-gaps * Implement a periodic vacate to address permanent D-state (uninterupptible wait) causing jobs to fail to complete * Always create a log file. If not specified put one in the output directory * Fix a problem with periodic vacate. * Up the periodic vacate time to 3 hrs * Found a job killing typo * Add time limits to post processing also * Don't save segments.txt file if no sgments founds because we don't know if it's an issue of not finding them or a valid not analyzable state. * disable periodic vacate to demo the problem. * Fix reported version in some utilities. Only update segments.txt if omicron is actually run. * Clarify relative imports. and add details to a few log messages * Resolve flake8 issues --------- Co-authored-by: Joseph Areeda <[email protected]>

tweak logging to better underst guardian channel usage-

duncanmmacleod · 2023-10-26T08:28:42Z

@areeda, I only see logging changes, and not anything related to concurrency or look back. Are there commits missing?

…on segfaults if environment variable not set

pep8speaks · 2023-10-26T17:54:48Z

Hello @areeda! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2023-12-04 21:53:41 UTC

codecov · 2023-10-26T17:58:59Z

Codecov Report

Attention: 14 lines in your changes are missing coverage. Please review.

Comparison is base (360a252) 35.15% compared to head (6167626) 33.97%.
Report is 1 commits behind head on master.

❗ Current head 6167626 differs from pull request most recent head cc9348e. Consider uploading reports for the commit cc9348e to get more accurate results

Files	Patch %	Lines
omicron/cli/process.py	0.00%	13 Missing ⚠️
omicron/parameters.py	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #173      +/-   ##
==========================================
- Coverage   35.15%   33.97%   -1.18%     
==========================================
  Files          30       30              
  Lines        2942     2938       -4     
==========================================
- Hits         1034      998      -36     
- Misses       1908     1940      +32

Flag	Coverage Δ
Conda	`33.97% <26.32%> (-1.18%)`	⬇️
Linux	`33.97% <26.32%> (-1.18%)`	⬇️
macOS	`33.97% <26.32%> (-1.18%)`	⬇️
python3.10	`33.97% <26.32%> (-1.18%)`	⬇️
python3.11	`33.97% <26.32%> (-1.18%)`	⬇️
python3.9	`33.98% <26.32%> (-1.18%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

areeda · 2023-10-26T18:03:27Z

Oops looks like the changes at line 744 in process have been previously merged
The remaining 3 failing tests are due to this bug in omicron https://git.ligo.org/virgo/virgoapp/Omicron/-/issues/81

duncanmmacleod · 2023-10-27T09:15:59Z

omicron/parameters.py

@@ -65,7 +65,7 @@ def __init__(self, version=None, defaults=dict(), **kwargs):
        if version is None:
            try:
                version = utils.get_omicron_version()
-            except KeyError:
+            except (KeyError, RuntimeError):


The get_omicron_version function, as far as I can tell, cannot raise a KeyError under any circumstances, so this should be a replacement rather than an append:

Suggested change

except (KeyError, RuntimeError):

except RuntimeError:

However, I'm not sure that we should be skipping this sort of error anyway.

When omicron version segfaults subprocess produces a runtime error.
Other omicron functions work.

duncanmmacleod · 2023-10-27T09:16:50Z

omicron/tests/test_parameters.py

+    # I disabled this test because the Omicron pfogram segfaults when
+    # it is run from pytest
+    # cp = ConfigParser()
+    # section = 'test'
+    # cp.add_section(section)
+    # cp.set(section, 'channels', 'X1:TEST-CHANNEL\nX1:TEST-CHANNEL_2')
+    # cp.set(section, 'flow', '10')
+    # cp.set(section, 'fhigh', '100')
+    # with tempfile.NamedTemporaryFile(suffix='.ini', mode='w') as f:
+    #     cp.write(f)
+    #     pars = OmicronParameters.from_channel_list_config(cp, section)
+    # assert pars.getlist('DATA', 'CHANNELS') == ['X1:TEST-CHANNEL',
+    #                                             'X1:TEST-CHANNEL_2']
+    # assert tuple(pars.getfloats('PARAMETER', 'FREQUENCYRANGE')) == (10., 100.)
+    pass


Can you raise an issue about this, it really shouldn't be segfaulting under any circumstances.

https://git.ligo.org/virgo/virgoapp/Omicron/-/issues/81 opened a while ago
We had a short discussion ib the Mattermost Omicron channel

areeda · 2024-09-09T16:27:16Z

This change inclued in other MR

Joseph Areeda and others added 13 commits September 20, 2022 15:26

Do not cross metric day boundaries.

73ceb29

minor doc changes

6b5403b

Fix a bug where an xml.gz file could get compressed again in merge-wi…

978cc76

…th-gaps

Resolve flake8 issues

3a2052f

Merge remote-tracking branch 'upstream/master'

9c9f35f

Update log format to use human readble date/time instead of gps

d7eb601

tweak logging to better underst guardian channel usage-

Merge remote-tracking branch 'upstream/master'

9d06142

remove old setup.cfg

4f0ab08

Merge remote-tracking branch 'upstream/master'

59840cc

Work vonpytest failures. The remaining errors are the result of omicr…

e57e810

…on segfaults if environment variable not set

missing blank line, from flake8

9d175ed

duncanmmacleod reviewed Oct 27, 2023

View reviewed changes

Joseph Areeda and others added 2 commits October 28, 2023 07:20

Merge remote-tracking branch 'upstream/master'

6167626

v2.0.8 merged

cc9348e

areeda closed this Sep 9, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Minor changes from big branch grd-failure #173

Minor changes from big branch grd-failure #173

areeda commented Oct 25, 2023

duncanmmacleod commented Oct 26, 2023

pep8speaks commented Oct 26, 2023 •

edited

Loading

codecov bot commented Oct 26, 2023 •

edited

Loading

areeda commented Oct 26, 2023

duncanmmacleod Oct 27, 2023

duncanmmacleod Oct 27, 2023

areeda Oct 27, 2023

duncanmmacleod Oct 27, 2023

areeda Oct 27, 2023

areeda commented Sep 9, 2024

Minor changes from big branch grd-failure #173

Minor changes from big branch grd-failure #173

Conversation

areeda commented Oct 25, 2023

duncanmmacleod commented Oct 26, 2023

pep8speaks commented Oct 26, 2023 • edited Loading

Comment last updated at 2023-12-04 21:53:41 UTC

codecov bot commented Oct 26, 2023 • edited Loading

Codecov Report

areeda commented Oct 26, 2023

duncanmmacleod Oct 27, 2023

Choose a reason for hiding this comment

duncanmmacleod Oct 27, 2023

Choose a reason for hiding this comment

areeda Oct 27, 2023

Choose a reason for hiding this comment

duncanmmacleod Oct 27, 2023

Choose a reason for hiding this comment

areeda Oct 27, 2023

Choose a reason for hiding this comment

areeda commented Sep 9, 2024

pep8speaks commented Oct 26, 2023 •

edited

Loading

codecov bot commented Oct 26, 2023 •

edited

Loading