Unknown injections handling in offline #4360

GarethCabournDavies · 2023-05-12T12:40:26Z

Allow the offline search to use injections which do not have an associated injection file.

For the use-case I am thinking of, we would be provided frame files with an unknown set of injections by an external team.

We would then use the alternative datafind settings on the config for the injected data, but use the original uninjected data for the background in the same way we do for known injections.

These would then show up as an additional minifollowup in the open box results, and a .hdf results file is made in the open box results directory which can be used for reporting results

Draft status is due to ongoing testing, this will be removed once the testing has completed and passed sensible checks

GarethCabournDavies · 2023-05-26T14:38:58Z

I had been wanting to allow in-frame injections which have a file as well, in case of supplied known injections, but I realised that can be done using #4363 and overriding the --injection-scale-factor option to be (close to) infinite for those injections, similar to single_template-inj_params_noinj plotting option, so no development is needed for that

spxiwh

I think I'm happy with this. Is there an example output page or test?

spxiwh · 2023-05-30T09:50:29Z

bin/workflows/pycbc_make_coinc_search_workflow

@@ -467,6 +467,9 @@ fore_xmlall = wf.make_foreground_table(workflow, combined_bg_file,
 fore_xmlloudest = wf.make_foreground_table(workflow, combined_bg_file,
                    hdfbank, rdir['open_box_result'], singles=insps,
                    extension='.xml', tags=["xmlloudest"])
+fore_hdfall = wf.make_foreground_table(workflow, combined_bg_file,
+                    hdfbank, rdir['open_box_result'], singles=insps,
+                    extension='.hdf', tags=["hdfall"])


This would appear to require changes to config files? Not sure I can see those?

Thanks for the pointer, missed that!
Added now, and running test workflow to generate some results pages. I'll add some injection generation / searching into the CI tests as well (similar to the live CI)

GarethCabournDavies · 2023-06-01T11:02:49Z

Note that changes since the approval will show up on GarethCabournDavies/pycbc@1f4991f...unknown_injections

The URL has updated now that I've rebased to master

GarethCabournDavies · 2023-06-15T08:53:29Z

Note: Current failure in the CI will be fixed by #4404

spxiwh · 2023-07-02T21:30:31Z

Poke: Still failing search test.

GarethCabournDavies · 2023-07-03T10:40:04Z

Searched through the logs and can't find any reasons why the coinc jobs for unknown injections are failing, just the transfers of the output file

GarethCabournDavies · 2023-07-04T11:31:17Z

Okay, the CI failures are due to huge injinj / fullinj / injfull jobs which are being booted I think - at least they are taking a lot of time and memory on my local runs using the examples config

I think that merging #4420 will fix the issues, so unfortunately, that becomes another dependency!

GarethCabournDavies · 2023-07-05T13:32:21Z

Results are currently showing that the injection run is using the full_data data. I think I have a solution, and am testing at the moment

spxiwh · 2023-07-10T08:55:52Z

Looks like there's now conflicts here. Poke me when this is ready!

…nalysed

GarethCabournDavies · 2023-07-11T11:17:05Z

Okay @spxiwh, this looks sensible on the results pages now - I thought the wrong data was being passed to the minifollowups, but it was me trying to overwrite rather than use a fresh directory

I'm slightly worried that the config file syntax becomes very complicated at this point, but I suppose this is a non-standard use-case

spxiwh

Thanks. I think I'm largely okay with this, do see some of the comments though.

bin/plotting/pycbc_page_foreground

bin/workflows/pycbc_make_offline_search_workflow

examples/search/analysis.ini

spxiwh · 2023-07-13T12:00:39Z

examples/search/gen.sh

+  plotting.ini \
+  executables.ini \
+  injections_minimal.ini \
+  injections_unknown.ini \


By how much does this slow down the example search workflow. While we want to test everything, the CI is becoming overstressed.

I can revert the changes to the CI after final approval, if that is best?

Handwaving discussion over time though:

the workflow additions are all parallel to already-running jobs

the generation of data with injections does take a few minutes of added time before the workflow starts, and is not parallel

The CI only has a single machine, so cannot parallelize. It's going to have to run all the jobs here one by one (or perhaps two-by-two). Therefore we do need to know how much extra time this is adding, and if this feature is "niche" enough to warrant this, given that there's probably many possibilities in workflow that we are not testing.

examples/search/plotting.ini

pycbc/io/hdf.py

spxiwh · 2023-07-13T13:41:14Z

pycbc/workflow/injection.py

+        elif injection_method == "IN_FRAME":
+            # IN_FRAME means that we will supply frames with the appropriate
+            # injections unknown to the analysis
+            inj_files.append(None)


My only substantial comment is that having a File be None is a hack. If anyone accidentally sent this file to a job, pegasus is going to [rightly] break. There doesn't seem to be a good way to handle this though ...

Yeah, though people shouldn't do that, there wont be a nice error message to say what they have done which is wrong either

spxiwh · 2023-07-17T09:39:03Z

Okay, I'm going to remove this from my "to-do" list now. Please do consider CI time, but merge this once you are happy and the CI tests (including the search workflow!) have completed. If you make any more substantive changes, explicitly poke me again.

tdent · 2023-07-26T10:09:41Z

@spxiwh is the use case for this going to come up in the near future to your knowledge? If so, we will want this to be on master

GarethCabournDavies · 2023-12-07T11:34:48Z

This will need a substantial re-think, as the use-case I've implemented here is unlikely to be used in the near future

What we are going to need is the case where injection frame files are supplied, and an injection definition file is also given, but we are not injecting them ourselves.

As a result, I will close, and think about how to adapt this for that use-case.

GarethCabournDavies added the offline search label May 12, 2023

GarethCabournDavies self-assigned this May 12, 2023

GarethCabournDavies changed the title ~~Draft: unknown injections handling in offline~~ Unknown injections handling in offline May 26, 2023

GarethCabournDavies force-pushed the unknown_injections branch from 100145f to 9f92ddf Compare May 26, 2023 14:42

GarethCabournDavies changed the title ~~Unknown injections handling in offline~~ Draft: Unknown injections handling in offline May 26, 2023

spxiwh approved these changes May 30, 2023

View reviewed changes

GarethCabournDavies force-pushed the unknown_injections branch from c25d95f to 8fc08e3 Compare May 31, 2023 13:22

GarethCabournDavies changed the title ~~Draft: Unknown injections handling in offline~~ Unknown injections handling in offline Jun 1, 2023

GarethCabournDavies force-pushed the unknown_injections branch from 3840827 to 43054a6 Compare June 13, 2023 12:09

GarethCabournDavies mentioned this pull request Jun 16, 2023

Get FFT options in pycbc_condition_strain #4404

Merged

GarethCabournDavies force-pushed the unknown_injections branch from 43054a6 to 95b8cb2 Compare June 27, 2023 09:11

GarethCabournDavies force-pushed the unknown_injections branch from cb04258 to 98a5f38 Compare July 4, 2023 13:25

GarethCabournDavies mentioned this pull request Jul 5, 2023

Allow the offline workflow to deal with blind injection sets #4345

Open

spxiwh self-requested a review July 10, 2023 08:55

GarethCabournDavies added 8 commits July 10, 2023 03:19

start working on unknown injections handling

7057341

Add unknown injections to CI

c4678bb

allow inections to use pycbc_page_foreground

c383b7f

allow exclusive for xml output

12a3020

xml fix

d7a3992

Use MKL backend for CI test

cb14557

FFTW good for bigger arrays 🤦

d1d2430

Fix error in injinj jobs for backgrounds

fef9006

GarethCabournDavies added 8 commits July 10, 2023 03:20

minifollowup needs foreground for unknown injections

6b26d5f

minifollowup details needed

e24c9ba

Codeclimate complaints

534ad15

Fix toactually use the injection data!

24b4845

Some fixes to data for injection run

658933e

commit for rebase

ab2db52

Undo testing changes

901ac50

Some changes to the ini files to allow the unknown injections to be a…

a8e9692

…nalysed

GarethCabournDavies force-pushed the unknown_injections branch from 98a5f38 to a8e9692 Compare July 10, 2023 16:06

spxiwh reviewed Jul 13, 2023

View reviewed changes

IH comments

a7642b6

GarethCabournDavies mentioned this pull request Sep 5, 2023

Use pregenerated injection frame file to perform pregenerated injections #4477

Open

GarethCabournDavies closed this Dec 7, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Unknown injections handling in offline #4360

Unknown injections handling in offline #4360

GarethCabournDavies commented May 12, 2023

GarethCabournDavies commented May 26, 2023 •

edited

Loading

spxiwh left a comment

spxiwh May 30, 2023

GarethCabournDavies May 31, 2023

GarethCabournDavies commented Jun 1, 2023 •

edited

Loading

GarethCabournDavies commented Jun 15, 2023

spxiwh commented Jul 2, 2023

GarethCabournDavies commented Jul 3, 2023

GarethCabournDavies commented Jul 4, 2023

GarethCabournDavies commented Jul 5, 2023

spxiwh commented Jul 10, 2023

GarethCabournDavies commented Jul 11, 2023

spxiwh left a comment

spxiwh Jul 13, 2023

GarethCabournDavies Jul 14, 2023

spxiwh Jul 17, 2023

spxiwh Jul 13, 2023

GarethCabournDavies Jul 14, 2023

spxiwh commented Jul 17, 2023

tdent commented Jul 26, 2023

GarethCabournDavies commented Dec 7, 2023

Unknown injections handling in offline #4360

Unknown injections handling in offline #4360

Conversation

GarethCabournDavies commented May 12, 2023

GarethCabournDavies commented May 26, 2023 • edited Loading

spxiwh left a comment

Choose a reason for hiding this comment

spxiwh May 30, 2023

Choose a reason for hiding this comment

GarethCabournDavies May 31, 2023

Choose a reason for hiding this comment

GarethCabournDavies commented Jun 1, 2023 • edited Loading

GarethCabournDavies commented Jun 15, 2023

spxiwh commented Jul 2, 2023

GarethCabournDavies commented Jul 3, 2023

GarethCabournDavies commented Jul 4, 2023

GarethCabournDavies commented Jul 5, 2023

spxiwh commented Jul 10, 2023

GarethCabournDavies commented Jul 11, 2023

spxiwh left a comment

Choose a reason for hiding this comment

spxiwh Jul 13, 2023

Choose a reason for hiding this comment

GarethCabournDavies Jul 14, 2023

Choose a reason for hiding this comment

spxiwh Jul 17, 2023

Choose a reason for hiding this comment

spxiwh Jul 13, 2023

Choose a reason for hiding this comment

GarethCabournDavies Jul 14, 2023

Choose a reason for hiding this comment

spxiwh commented Jul 17, 2023

tdent commented Jul 26, 2023

GarethCabournDavies commented Dec 7, 2023

GarethCabournDavies commented May 26, 2023 •

edited

Loading

GarethCabournDavies commented Jun 1, 2023 •

edited

Loading