Adding new parameters to site.py and adding a new gsim

[Prajakta-Jadhav-25]

This PR adds new soil and topographical parameters useful for predicting lateral spread displacements.
This PR also adds new gsim, Youd et al. (2002), for estimating lateral spread displacements.

[micheles]

This is all good, but you should also add the code using the new parameters ;-)

[Prajakta-Jadhav-25]

Thanks Michele. I will open a new pull request submitting the code for corresponding GSIM using these site parameters along with the other related files.

[micheles]

The way to implement new GMPEs has changed drastically in the last weeks. You need to rewrite your GMPE to the compute API (follow any example in the current hazardlib).

[Prajakta-Jadhav-25]

The GMPE has been updated in accordance with the other GMPEs in hazardlib. Kindly, let me know if I am missing something.

[micheles]

You are missing everything. Since one month ago GMPE classes do no have methods except a "compute" method. See for instance the implementation of https://github.com/gem/oq-engine/blob/master/openquake/hazardlib/gsim/raghukanth_iyengar_2007.py. You need to update your version of hazardlib. See for instance https://www.earthdatascience.org/courses/intro-to-earth-data-science/git-github/github-collaboration/update-github-repositories-with-changes-by-others/

[Prajakta-Jadhav-25]

As you rightly pointed out, my hazardlib wasn't updated. The shared links were quite useful.

I have updated the GMPE file in accordance with the current version. However, let me know, if I am still missing something.

Thanks,
Prajakta

[micheles]

Now it is much better. There still a few issues, though. The notation __XXX__ in Python is reserved for the internals, it is a bad idea to use it. It is much better instead use the names that we are using everywhere else, like _compute_magnitude_term, _compute_distance_term, etc.

[Prajakta-Jadhav-25]

I have updated the function names. Let me know other issues in the file. I'll work on them.

[mmpagani]

Please remove from your PR the tables of Lanzano and Kuehn. Also, I put a couple of notes that I kindly ask to be addressed.

[mmpagani]

I do not understand what this part of the code does. The freeface ratio is a site-specific coefficient. So depending on the value of this param for each site C and ctx.slope should be defined accordingly. However, with ctx.freeface_ratio.all() you check if all the sites have a value of this parameter == 0 and then apply a correction.

[Prajakta-Jadhav-25]

Hi Marco,

PFA herewith the response to this query. Let us know, if any modifications further are to be incorporated or if we are missing something.

Thanks,
Explanation for the following lines of the code_sept 12 2021.pdf

[Prajakta-Jadhav-25]

Hello Marco,

May we know the next steps to be taken in this pull request.

Thanks,
Prajakta

[Prajakta-Jadhav-25]

Please remove from your PR the tables of Lanzano and Kuehn. Also, I put a couple of notes that I kindly ask to be addressed.
I have deleted Lanzano and Kuehn tables from PR. Let me know, if its not done as suggested.

[Prajakta-Jadhav-25]

Hello Marco and Michele,

We have addressed your comments. May we know the next steps to be taken in this pull request.

Thanks,
Prajakta

[micheles]

The tests are broken, therefore this is a no go:

________________________ YoudEtAl2002TestCase.test_mean ________________________
hazardlib/tests/gsim/youd_etal_2002_test.py:30: in test_mean
    self.check('YOUD04/Youd2004_MEAN.csv',
hazardlib/tests/gsim/utils.py:232: in check
    raise ValueError(msg)
E   ValueError: {'out_type': 'MEAN', 'imt': 'PGD', 'expected': 0.04225547599999999, 'got': 0.0, 'discrep_percent': 100.0, 'F_15': 5.0, 'D50_15': 0.3, 'freeface_ratio': 20.0, 'repi': 1.00124922, 'T_15': 15.0, 'slope': 0.0, 'hypo_depth': 10.0, 'mag': 4.5}

[Prajakta-Jadhav-25]

Hi Marco and Michele:

Thanks for working on this pull request.

I wonder why the tests are failing. Are the site parameters 'slope', 'freeface_ratio', 'T_15', 'F_15', 'D50_15' being included in the site.py and other related files?

[mmpagani]

Hi Prajakta, Please have a look at the link below. Are tests green on your machine? https://github.com/gem/oq-engine/runs/4809232021?check_suite_focus=true#step:5:241 Marco

…

Hi Marco and Michele: Thanks for working on this pull request. I wonder why the tests are failing. Are the site parameters 'slope', 'freeface_ratio', 'T_15', 'F_15', 'D50_15' being included in the site.py and other related files? -- Reply to this email directly or view it on GitHub: #7020 (comment) You are receiving this because you commented. Message ID: ***@***.***>

[Prajakta-Jadhav-25]

Hi Marco:

I checked the mentioned link. Yes, the tests did not run on my machine as well and showed the same error.
This is possibly because, the parameters 'slope', 'freeface_ratio', 'T_15', 'F_15', 'D50_15' are new and I feel just defining them in site.py is not enough.
When running the code in developer OQ version, as I mentioned in our meeting a while back, I did not define these parameters but instead used the existing parameters like cti, dw and so on and made changes in another file viz., oqvalidation. After incorporating these changes, as I showed you in the meeting, the tests were running fine. However, I learnt that you weren't comfortable in making changes in the oqvalidation file.

So, we need to define these new parameters, viz., 'slope', 'freeface_ratio', 'T_15', 'F_15', 'D50_15' in site.py file and ensure that these are read in the Youd et al. gsim file. In order to do this, we might have to change other files. As a developer, can you suggest, which file needs to be changed?

Thanks,

Prajakta

[mmpagani]

I do not think it depends on the fact that you are using new site terms. The error you get is generated by the tests defined in the verification tables. The error message explains that for that combination of parameters your model gives 0.0 while according to the test it should give 0.04225547599999999. You should check your model.

[Prajakta-Jadhav-25]

Hi Marco:

It makes me think why the model is in fact calculating 0.0 as the output, if it is able to take the values for parameters. I tried to run the tests on my PC using the same files that I have uploaded on Github in a newer version of OQ and I am getting an attribute error (Check attached image 'error_test'). I have also included image of a successful test ('success_test'), which was run in older version of OQ and using already defined parameters ('dw' in the place of free-face ratio). I can look into this in more detail, however, any suggestions on what might be going wrong would be of great help!

[micheles]

I will look at the PR tomorrow, but today we have the release 3.13 to make ;-)

[Prajakta-Jadhav-25]

Sure Michele :) 👍

[micheles]

The error is clear: in the file Youd2004_MEAN.csv the slope parameter has value 0 for some magnitudes (for instance mag=4.5); then you have a site term C["c4"] * np.log10(sites.slope) producing a result of -inf. Then exp(-inf)=0 and that is the reason that you are getting 0 for the mean value and not the expected one.

[Prajakta-Jadhav-25]

Hi Michele: Thanks for looking into this. I don't think that is resulting in zero value, because, I have included these lines, if ctx.freeface_ratio.all() == 0.0:
C = self.COEFFS_SLOPE[imt]
else:
C = self.COEFFS_FREEFACE[imt]
ctx.slope = ctx.freeface_ratio

The idea is, either of slope or freeface_ratio should be non-zero. As a result, if slope=0, then slope=freeface_ratio and the calculation will be done using COEFFS_FREEFACE table. Do you think, there is issue with this logic or syntax, because, this logic has worked fine in the previous version of OQ as shown through successful test result in previous conversation?

[micheles]

The logic here is implemented incorrectly:

 if ctx.freeface_ratio.all() == 0.0:
   C = self.COEFFS_SLOPE[imt]
else:
  C = self.COEFFS_FREEFACE[imt]
  ctx.slope = ctx.freeface_ratio

I am going to close this PR and reopen it with the logic implemented correctly.