v2_ssp_func
Claudio,
Here is my first attempt to reorganize sourcespec in a way that it can be run as a function. I don't think I wrote more than 5 new lines, but most of the changes consisted of moving existing code to new functions:
- in
ssp_setup.pyI added a newget_outdir_pathfunction - in
ssp_read_traces.pyI added newread_station_inventory,read_event_and_picks,augment_event,augment_tracesandselect_componentsfunctions - in
source_spec.py, I added newssp_runandssp_outputfunctions
I haven't been able to test it yet. I will need to modify my own code quite substantially before that will be possible, but I will first try writing a simple notebook to see if I can make it work.
You will probably want to organize the new functions in different files. Also,don't hesitate if you want to change the names of these new functions.
In some places, I added some TODO lines regarding alternative decisions that could be taken or questions that I have.
If you have time, please have a look and let me know if this makes sense.
Kris
Thanks Kris!
Only five additional lines is perfect 😆 . I think we should keep these pull requests sufficiently small to be able to be easily reviewed: more pull requests will follow 😉
Not sure I'll have time this week to review the PR, but I'll certainly work on it next week!
Hi Kris,
I made some modifications:
- Improved docstrings
- Necessary code modifications to run in CLI
The main problem was that config.event is not available before calling augment_event(), so I choose to explicitly pass the eventid to many functions.
I think that we should reflect on whether we want event to be within config: this is the only input data that is treated this way (traces, metadata, spectra are all explicitly passed to functions).
Anyway, we can keep this change for later, since it is quite a major one.
Other point: it is not clear to me why read_station_inventory() is needed, since it just calls read_station_metadata().
Finally, to test the code, you have to force-pull, since I rebased.
Note that all the squashme- commits are supposed to be squashed within the previous one (I will do it just before merging this PR).
Claudio, I force-pulled the branch and my notebook still works!
- I missed the potential problem with
config.eventwhen running the CLI, but indeed we should investigate if this can be handled differently in the relevant functions. - There is indeed no need for the
read_station_inventoryfunction. - In the mean time, I added a cell to the notebook to run the
ssp_outputfunction using a temporary directory with all save options deactivated andconfig.plot_show = True. I obtain plots of the traces, the fitted spectra, the spectral weights, the stacked spectra and the boxplot. Only 3 files are written:<event_id>.residuals.hdf5,<event_id>.ssp.outand<event_id>.ssp.yaml. Would you consider configuration options for each type of plot and output? - I tried changing the logging level, but it did not work. We will probably have to work on the logging setup, to make it work irrespective of whether it is written to a file or not.
Thanks for the feeback.
I'm going to merge this one and start working on reorganize the setup and read input part of the code into subdirectories.
I'll also take care of the two Config().__init__() fixes mentioned here.
It would be nice if you could send me a working script or notebook to test the API all along the modifications 😉
Claudio, thanks. To avoid my specific functions to read event data, phase picks and waveform data, I can write everything into a ASDF file, which is easier to share. I need to work out what would be the easiest way to read all pieces of information from the ASDF file, and maybe add support for that in sourcespec. What would be the easiest way to share the notebook and data file(s)?
What would be the easiest way to share the notebook and data file(s)?
For the moment, just send me an email or a transfer link? We will see later what to include as an offical example 😉
Kris, please take a look at these two commits:
- https://github.com/SeismicSource/sourcespec/commit/5b5a0fa200dcbcd63e2ac0b1944908f91ff49743 : in-place
select_components()(I wonder ifselect_components()could be merged intoaugment_traces()) - https://github.com/SeismicSource/sourcespec/commit/233222f967523dece7a766a502b2f52988c24119 :
configsubmodule renamed tosetupand content ofssp_setupy.pysplit in three files
This will require some adjustment on your side 😉
I further moved all the input functions into an input subdirectory (submodule).
https://github.com/SeismicSource/sourcespec/commit/098f3f2cd83e3beec73c2dd427033946b7dee3ff
Also, I moved select_components() into augment_traces()
You are doing quite a bit of refactoring, but it looks good! I only had to modify one import line in the notebook.