Adding more MWSRs

[jpa198]

Adding Thingbaijam, Murotani, Blaser, Allen and Strasser models. Have put Allen and Strasser in separate files to reduce how big the MWSR file is getting

[jpa198]

Two questions:

Any thoughts on how to extend this when we get more MWSRs? Half of the functions in the file have the year in them and half don't? What's the preference?

I think the functions like get_area could be simplified a bit if the mwsr functions took in a Fault object instead of just the magnitude/other variables (looking at you dip dependent leonard). Then a dict of MWSR enum to function could be used as a look up table. Generally though I think adding MWSRs is always going to be a faf as it's lots of small equations. A class with a base class for each could be a way to standardise the attributes/functions available for each model. Having an entire class for each seems like overkill though

Good catch, I'll add the year to the ones I'm adding. Leonard 2010 potentially being in this batch prompted that push.

[jasonmotha]

Yeah, some of them have a standard form though. So I was potentially thinking you could go empirical styles and have tables of coeffs. But that's also overkill.

It's probably just easier leaving it as a faff, it doesn't make it harder to implement new ones or call the functions.

[jasonmotha]

In terms of verification, I presume a lot of it is copied and pasted so generating graphs of area / mw is overkill?

[jpa198]

In terms of verification, I presume a lot of it is copied and pasted so generating graphs of area / mw is overkill?

It is copy pasted, probably not a bad idea though still, just as an idiot check

[jpa198]

Created a wiki page for magnitude scaling relationships: https://wiki.canterbury.ac.nz/display/QuakeCore/Magnitude+scaling+relationships Has some validation of the models too

[jpa198]

type hint on l29 is cooked.

What's wrong with it? It's supposed to be a string as Fault doesn't exist in the file and I'd have to import it just for the type hint

[jasonmotha]

type hint on l29 is cooked.

What's wrong with it? It's supposed to be a string as Fault doesn't exist in the file and I'd have to import it just for the type hint

I think importing it is a better idea, as it makes it very easy to see what attributes a fault object should have.

[jpa198]

type hint on l29 is cooked.

What's wrong with it? It's supposed to be a string as Fault doesn't exist in the file and I'd have to import it just for the type hint

I think importing it is a better idea, as it makes it very easy to see what attributes a fault object should have.

2 things. I also don't want to deal with the circular import for a type and Fault is an abstract base class, with the attributes available depending on if you have a fault of type 1, 2, 3 or 4

https://github.com/ucgmsim/Pre-processing/blob/master/srf_generation/Fault.py

[jasonmotha]

2 things. I also don't want to deal with the circular import for a type and Fault is an abstract base class, with the attributes available depending on if you have a fault of type 1, 2, 3 or 4

https://github.com/ucgmsim/Pre-processing/blob/master/srf_generation/Fault.py

Oh yep. The circular import is definitely a show stopper. Might be worth fixing it later though - like having the abstract base class in a separate file (with a whole bunch of caveats around that)

[jasonmotha]

If you have the plots, it would be good to attach them to this PR.

[jpa198]

If you have the plots, it would be good to attach them to this PR.

They're on the documentation wiki page I linked https://wiki.canterbury.ac.nz/display/QuakeCore/Magnitude+scaling+relationships

[joelridden]

Two questions: Any thoughts on how to extend this when we get more MWSRs? Half of the functions in the file have the year in them and half don't? What's the preference?

I think the functions like get_area could be simplified a bit if the mwsr functions took in a Fault object instead of just the magnitude/other variables (looking at you dip dependent leonard). Then a dict of MWSR enum to function could be used as a look up table. Generally though I think adding MWSRs is always going to be a faf as it's lots of small equations. A class with a base class for each could be a way to standardise the attributes/functions available for each model. Having an entire class for each seems like overkill though

Good catch, I'll add the year to the ones I'm adding. Leonard 2010 potentially being in this batch prompted that push.

I agree with reducing the get_area like functions by using a mapping with a dict of MWSR enum to function. That seems like a good idea to reduce the amount of elif's especially if the amount of MWSRs are going in to increase in future.

[jpa198]

Two questions: Any thoughts on how to extend this when we get more MWSRs? Half of the functions in the file have the year in them and half don't? What's the preference?

I think the functions like get_area could be simplified a bit if the mwsr functions took in a Fault object instead of just the magnitude/other variables (looking at you dip dependent leonard). Then a dict of MWSR enum to function could be used as a look up table. Generally though I think adding MWSRs is always going to be a faf as it's lots of small equations. A class with a base class for each could be a way to standardise the attributes/functions available for each model. Having an entire class for each seems like overkill though Good catch, I'll add the year to the ones I'm adding. Leonard 2010 potentially being in this batch prompted that push.

I agree with reducing the get_area like functions by using a mapping with a dict of MWSR enum to function. That seems like a good idea to reduce the amount of elif's especially if the amount of MWSRs are going in to increase in future.

It would be more elegant to use the dict, the main issue is either duplicating every function to provide a Fault compatible wrapper, or to drop raw access to the formulas, and require everything to be passed in via Fault object.

Using Fault objects only is a bit too enterprise for us/makes casual use harder, and I feel like making a wrapper for each function is just moving the faff from the Mw_2_area function to a dozen two line functions without actually removing it

[jpa198]

Two questions: Any thoughts on how to extend this when we get more MWSRs? Half of the functions in the file have the year in them and half don't? What's the preference?

I think the functions like get_area could be simplified a bit if the mwsr functions took in a Fault object instead of just the magnitude/other variables (looking at you dip dependent leonard). Then a dict of MWSR enum to function could be used as a look up table. Generally though I think adding MWSRs is always going to be a faf as it's lots of small equations. A class with a base class for each could be a way to standardise the attributes/functions available for each model. Having an entire class for each seems like overkill though Good catch, I'll add the year to the ones I'm adding. Leonard 2010 potentially being in this batch prompted that push.

I agree with reducing the get_area like functions by using a mapping with a dict of MWSR enum to function. That seems like a good idea to reduce the amount of elif's especially if the amount of MWSRs are going in to increase in future.

It would be more elegant to use the dict, the main issue is either duplicating every function to provide a Fault compatible wrapper, or to drop raw access to the formulas, and require everything to be passed in via Fault object.

Using Fault objects only is a bit too enterprise for us/makes casual use harder, and I feel like making a wrapper for each function is just moving the faff from the Mw_2_area function to a dozen two line functions without actually removing it

Actually, having the dict for the magnitude only ones would work, then handling the other 2 separately