search

California-Planet-Search / radvel

General Toolkit for Modeling Radial Velocity Data
http://radvel.readthedocs.io
MIT License
57 stars 52 forks source link

Support more celerite kernels ? #353

Open vandalt opened 2 years ago

vandalt commented 2 years ago

Hello, I have a few use cases where celerite kernels other than the Quasi-Periodic one currently implemented in RadVel (mainly the SHO and Matern 3/2 ones) could be useful. I have implementations that seem to be working (it replicates what the current CeleriteKernel is doing but with different terms). I could send a PR with this if you are interested.

I also have somewhat related questions:

  1. Are there plans to move to celerite2 ? Celerite seems to be in maintenance mode and celerite2 has more active development, but it only supports Python 3.6 and later.
  2. What Python versions are supported by RadVel (i.e. if I were to send the PR above which versions should it support) ?
  3. What maximum line length are you using as a formatting guideline ? I could not find this in the docs.

Thanks!

bjfultn commented 2 years ago

  1. Yes, definitely send over a PR more kernels are always good.

  2. I was actually unaware of celerite2 I'd have to do some research to see what it would take to transition over but I'm certainly not against it. The Python 3.6 requirement is not a problem as I no longer support Python 2.X.

  3. There are no strict requirements but I almost always work with Python >= 3.7.4. Python 2 support for radvel has been 'use at your own risk' for quite awhile (1.2.x) and I don't think it will work these days but I haven't tested.

  4. PEP8 standard is 80 characters. This may not be practiced 100% of the time in the existing code but it is the goal. 120 is fine for documentation lines if you need it.