SimpleOrbit updates

[icweaver]

This PR updates the tests for SimpleOrbit, adds support for negative times, and re-defines when z is positive/negative to be in line with the new criteria introduced in ad99636:

Transits.jl/src/Transits.jl

Lines 82 to 91 in eaa263b

	function compute(ld::AbstractLimbDark, orbit::AbstractOrbit, t, r)
	coords = Orbits.relative_position(orbit, t)
	# make sure los is in front of star
	if coords[3] > 0
	μ = sqrt(coords[1]^2 + coords[2]^2)
	return compute(ld, μ, r)
	else
	return one(eltype(coords))
	end
	end

[codecov]

Codecov Report

Merging #22 (b82295f) into main (eaa263b) will increase coverage by 0.11%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main      #22      +/-   ##
==========================================
+ Coverage   90.61%   90.72%   +0.11%     
==========================================
  Files          12       12              
  Lines         959      960       +1     
==========================================
+ Hits          869      871       +2     
+ Misses         90       89       -1     

Impacted Files	Coverage Δ
src/orbits/simple.jl	100.00% <100.00%> (+3.12%)	⬆️
src/Transits.jl	88.88% <0.00%> (+1.38%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update eaa263b...b82295f. Read the comment docs.

[icweaver]

I'm currently setting z to 1 if the planet is on the half of its orbit nearest to us and 0 otherwise, but from a code re-use standpoint (and for secondary eclipses!) it would be nice if we could maintain the same condition that it is only 1 while in transit. I will take a look at this next

[icweaver]

Huh, just tried it out in exoplanet, and apparently the clipping is not just on our end:

I was probably just getting too caught-up in trying to make everything look like our previous transit curves. Since exoplanet has a test comparing their simple orbit to a Keplerian one, I think we should be ok. I'll switch back to what we had before with the working version of SimpleOrbit we have now, and add in that test as well (after we settle on an API)

Further discussion at exoplanet-dev/exoplanet#148

[mileslucas]

I'm a bit confused- besides the mod vs %/rem change, everything is all good, right?

The reason why you're seeing the cutoff is because that's at exactly 1/4 the period (right?). Exoplanet basically says "you can't have a duration > period/2, but we don't stop you from trying" and I think keeping consistency with them there is good enough. I only wrote the SimpleOrbit because I wanted to get the limb darkening curves started and didn't want to make Keplerian orbits yet :)

[icweaver]

I'm a bit confused- besides the mod vs %/rem change, everything is all good, right?

The reason why you're seeing the cutoff is because that's at exactly 1/4 the period (right?). Exoplanet basically says "you can't have a duration > period/2, but we don't stop you from trying" and I think keeping consistency with them there is good enough. I only wrote the SimpleOrbit because I wanted to get the limb darkening curves started and didn't want to make Keplerian orbits yet :)

Yea, I think that is reasonable and that things are looking good! Definitely interested to compare to our KeplerianOrbit curves =]

[icweaver]

It looks like our coverage dropped just a smidge. Working on that now

[icweaver]

Ok, things are looking green 👍🏾

[mileslucas]

Thanks @icweaver