Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Minor changes from big branch grd-failure #173

Closed
wants to merge 17 commits into from
Closed
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 15 additions & 8 deletions omicron/cli/process.py
Original file line number Diff line number Diff line change
Expand Up @@ -472,7 +472,7 @@

logger.debug("Command line args:")
for arg in vars(args):
logger.debug(f'{arg} = {str(getattr(args, arg))}')
logger.debug(f' {arg} = {str(getattr(args, arg))}')

Check warning on line 475 in omicron/cli/process.py

View check run for this annotation

Codecov / codecov/patch

omicron/cli/process.py#L475

Added line #L475 was not covered by tests

# validate command line arguments
if args.ifo is None:
Expand Down Expand Up @@ -644,9 +644,12 @@
pass

if statechannel:
logger.debug("State channel = %s" % statechannel)
logger.debug("State bits = %s" % ', '.join(map(str, statebits)))
logger.debug("State frametype = %s" % stateft)
logger.debug(f"State channel {statechannel}")
if statebits == 'guardian':
logger.debug(f"State bits {statebits}")

Check warning on line 649 in omicron/cli/process.py

View check run for this annotation

Codecov / codecov/patch

omicron/cli/process.py#L647-L649

Added lines #L647 - L649 were not covered by tests
else:
logger.debug("State bits = %s" % ', '.join(map(str, statebits)))
logger.debug(f"State frametype {stateft}")

Check warning on line 652 in omicron/cli/process.py

View check run for this annotation

Codecov / codecov/patch

omicron/cli/process.py#L651-L652

Added lines #L651 - L652 were not covered by tests

# parse padding for state segments
if statechannel or stateflag:
Expand Down Expand Up @@ -674,7 +677,7 @@
# -- set directories ------------------------------------------------------

rundir.mkdir(exist_ok=True, parents=True)
logger.info(f"Using run directory\n{rundir}")
logger.info(f"Using run directory: {rundir}")

Check warning on line 680 in omicron/cli/process.py

View check run for this annotation

Codecov / codecov/patch

omicron/cli/process.py#L680

Added line #L680 was not covered by tests

cachedir = rundir / "cache"
condir = rundir / "condor"
Expand Down Expand Up @@ -729,7 +732,8 @@
end = data.get_latest_data_gps(ifo, frametype) - padding

try: # start from where we got to last time
start = segments.get_last_run_segment(segfile)[1]
last_run_segment = segments.get_last_run_segment(segfile)
start = last_run_segment[1]

Check warning on line 736 in omicron/cli/process.py

View check run for this annotation

Codecov / codecov/patch

omicron/cli/process.py#L735-L736

Added lines #L735 - L736 were not covered by tests
except IOError: # otherwise start with a sensible amount of data
if args.use_dev_shm: # process one chunk
logger.debug("No online segment record, starting with "
Expand All @@ -740,9 +744,10 @@
"4000 seconds")
start = end - 4000
else:
logger.debug("Online segment record recovered")
logger.debug(f"Online segment record recovered: {last_run_segment[0]} - {last_run_segment[1]}")

Check warning on line 747 in omicron/cli/process.py

View check run for this annotation

Codecov / codecov/patch

omicron/cli/process.py#L747

Added line #L747 was not covered by tests
elif online:
start, end = segments.get_last_run_segment(segfile)
logger.debug(f"Online segment record recovered: {start} - {end}")

Check warning on line 750 in omicron/cli/process.py

View check run for this annotation

Codecov / codecov/patch

omicron/cli/process.py#L750

Added line #L750 was not covered by tests
else:
start, end = args.gps
start = int(start)
Expand Down Expand Up @@ -785,9 +790,11 @@
if (online and statechannel) or (statechannel and not stateflag) or (
statechannel and args.no_segdb):
logger.info(f'Finding segments for relevant state... from:{datastart} length: {dataduration}s')
logger.debug(f'For segment finding: online: {online}, statechannel: {statechannel}, '

Check warning on line 793 in omicron/cli/process.py

View check run for this annotation

Codecov / codecov/patch

omicron/cli/process.py#L793

Added line #L793 was not covered by tests
f'stateflag: {stateflag} args.no_segdb: {args.no_segdb}')
seg_qry_strt = time.time()
if statebits == "guardian": # use guardian
logger.debug(f'Using guardian for {statechannel}: {datastart}-{dataend}')
logger.debug(f'Using guardian for {statechannel}: {datastart}-{dataend} ')

Check warning on line 797 in omicron/cli/process.py

View check run for this annotation

Codecov / codecov/patch

omicron/cli/process.py#L797

Added line #L797 was not covered by tests
segs = segments.get_guardian_segments(
statechannel,
stateft,
Expand Down
5 changes: 3 additions & 2 deletions omicron/log.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,9 @@ def filter(self, record):
class Logger(logging.Logger):
"""`~logging.Logger` with a nice format
"""
FORMAT = ('[{bold}%(name)s{reset} %(gpstime)d] %(levelname)+19s %(filename)s:%(lineno)d: '
FORMAT = ('[{bold}%(name)s{reset} %(asctime)s] %(levelname)+19s %(filename)s:%(lineno)d: '
'%(message)s'.format(bold=BOLD_SEQ, reset=RESET_SEQ))
log_file_date_format = '%m-%d %H:%M:%S'

def __init__(self, name, level=logging.DEBUG):
try:
Expand All @@ -87,7 +88,7 @@ def __init__(self, name, level=logging.DEBUG):
logging.Logger.__init__(self, name, level=level)

# set up handlers for WARNING and above to go to stderr
colorformatter = ColoredFormatter(self.FORMAT)
colorformatter = ColoredFormatter(self.FORMAT, datefmt=self.log_file_date_format)
stdouthandler = logging.StreamHandler(sys.stdout)
stdouthandler.setFormatter(colorformatter)
stderrhandler = logging.StreamHandler(sys.stderr)
Expand Down
2 changes: 1 addition & 1 deletion omicron/parameters.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@
if version is None:
try:
version = utils.get_omicron_version()
except KeyError:
except (KeyError, RuntimeError):

Check warning on line 68 in omicron/parameters.py

View check run for this annotation

Codecov / codecov/patch

omicron/parameters.py#L68

Added line #L68 was not covered by tests
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

version = utils.OmicronVersion(const.OMICRON_VERSION)
self.version = version
self._set_defaults()
Expand Down
2 changes: 1 addition & 1 deletion omicron/tests/test_log.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,4 +44,4 @@ def test_logger():

# test that the formatter prints the correct thing
outhandler = logger.handlers[0]
assert re.match('.*TEST.*\\d{10}.*DEBUG.*FILE.*test message', outhandler.format(record))
assert re.match('.*TEST.*\\d{2}-\\d{2}\\s+\\d{2}:\\d{2}.*DEBUG.*FILE.*test message', outhandler.format(record))
27 changes: 15 additions & 12 deletions omicron/tests/test_parameters.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,18 +94,21 @@ def test_validate_parameters(pars):


def test_from_channel_list_config():
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.)
# 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
Comment on lines +97 to +111
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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



def test_read_ini(pars):
Expand Down
Loading