Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

S40RTS and SAVANI initial temperature models use "vs to density scaling" as parameter names #760

Closed
bangerth opened this issue Feb 19, 2016 · 4 comments

Comments

@bangerth
Copy link
Contributor

We almost always use parameter names whose first letter is capitalized. In this case I think there isn't even much of an argument that it might be a symbol because people will probably have an easier time reading Vs over vs when thinking of the shear wave velocity.

It should be possible to rename these parameters in a backward compatible way by using the parameter aliases that were introduced a while back.

@Shangxin-Liu
Copy link
Contributor

Just recalled this discussion from Newsletter on mailing list. I also changed the vs to Vs in the last commit patch controlling the max order. @bangerth

@bangerth
Copy link
Contributor Author

bangerth commented Mar 4, 2016

OK. In general, keep each patch self-contained and to one issue. In other words, the correct procedure for the future would be to have one patch for the "vs vs. Vs" issue, and one for the issue discussed in #759 .

@bangerth bangerth added this to the 1.4 milestone Apr 9, 2016
@gassmoeller
Copy link
Member

I think this issue was solved by #759, and can be taken of the list for 1.4 . I will close it for now. If someone objects, feel free to reopen.

@Shangxin-Liu
Copy link
Contributor

Yes. This has been solved.

On Wed, May 4, 2016 at 8:36 AM, Rene Gassmöller notifications@github.com
wrote:

I think this issue was solved by #759
#759, and can be taken of the
list for 1.4 . I will close it for now. If someone objects, feel free to
reopen.


You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#760 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants