Cyberface / pycbc

Python CBC Tools
GNU General Public License v3.0
1 stars 0 forks source link

Notes on updating bank verifier #2

Closed Cyberface closed 6 years ago

Cyberface commented 6 years ago
Cyberface commented 6 years ago

From Ian

Okay, that needs a couple of changes, but then should be good. The problem is basically the config file. This option should be given like this one:

https://github.com/gwastro/pycbc/blob/master/pycbc/workflow/jobsetup.py#L613

(as a "file_input_option" ... which would then be supplied in the main config file as:

```[create_injections]
config-file = /path/to/config/file # Http paths also work (ie. for files in git repos)
other-options =
...```

Then remove https://github.com/gwastro/pycbc/blob/master/pycbc/workflow/jobsetup.py#L1659

Possibly also remove the seed option below it. It make make sense for that also to be supplied in the main config file.

.... All make sense?
pycbc/workflow/jobsetup.py:1659
gwastro/pycbcAdded by GitHub
Then all is needed is for the main workflow to choose between the two exes, possibly based on exe name??
There's a few examples of that here:

https://github.com/gwastro/pycbc/blob/master/pycbc/workflow/jobsetup.py#L79

Although it can be messy if different exes take different sets of options
pycbc/workflow/jobsetup.py:79
```def select_matchedfilter_class(curr_exe):```
gwastro/pycbcAdded by GitHub

Sebastian Khan [9:25 AM]
I don't understand why you link to this line https://github.com/gwastro/pycbc/blob/master/pycbc/workflow/jobsetup.py#L613
and do you want to be able to support both ways of doing bank verification (old [injspinj] and new [pycbc])

Ian Harry [9:26 AM]
It was the first example of something using "file_input_option" that I could see
And yes, it would be good to support both injection codes ... At least for now

Sebastian Khan [9:26 AM]
Is this line https://github.com/gwastro/pycbc/blob/master/pycbc/workflow/jobsetup.py#L613 saying "This is the require input file option"?

Ian Harry [9:28 AM]
Yeah, basically I mean at:

https://github.com/gwastro/pycbc/blob/master/pycbc/workflow/jobsetup.py#L1624

along with `current_retention_level`
add
```file_input_options = ['--config-file']```
and then remove other references to `config-file` in the setup code below
pycbc/workflow/jobsetup.py:1624
    ```current_retention_level = Executable.ALL_TRIGGERS```
gwastro/pycbcAdded by GitHub
That attribute tells the code that if this option is given it is a file and should be treated as such internally.

Sebastian Khan [9:29 AM]
ah ok - that level of python coding is a bit above mine I guess.
I will give it a go! Cheers!