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

Compute the position and velocity of the spacecraft #122

Merged
merged 50 commits into from
Sep 17, 2021
Merged

Compute the position and velocity of the spacecraft #122

merged 50 commits into from
Sep 17, 2021

Conversation

ziotom78
Copy link
Member

@ziotom78 ziotom78 commented Jul 6, 2021

When finished, this PR will implement a module that will simulate the orbit of the LiteBIRD spacecraft around the L2 point. It will be used to simulate the observability of Solar System objects as well as the computation of the solar/orbital dipole.

  • Define DEFAULT_COORDINATE_SYSTEM in a separate module (coordinates), so that it can be referenced every time we use astropy.coordinates;
  • Make litebird_sim/scanning.py use DEFAULT_COORDINATE_SYSTEM instead of hardwiring the coordinate system in the code
  • Implement compute_l2_pos_and_vel (compute the location and the velocity of the L2 point)
  • Implement compute_lissajous_pos_and_vel (compute the location and the velocity of the spacecraft wrt the L2 point)
  • Add the SpacecraftOrbit dataclass
  • Harmonize the compute_l2_pos_and_vel and compute_lissajous_pos_and_vel with the Observation class, so that the caller can know the position and velocity of the spacecraft at the time of an observation
  • Implement tests
  • Update the documentation

@ziotom78 ziotom78 marked this pull request as draft July 6, 2021 10:59
@ziotom78 ziotom78 self-assigned this Jul 6, 2021
@ziotom78 ziotom78 changed the title Add support for solar/orbital dipole Compute the position and velocity of the spacecraft Jul 6, 2021
@ziotom78
Copy link
Member Author

I have made some modifications to dipole.py to make sure that Numba is able to properly optimize the for loops. At the moment I decided to adopt a quick way to compute the velocities at the same sampling frequencies as the TOD, i.e., I just used numpy.interp to create a new N×3 array containing the N velocities, but the code is designed so that the interface is not going to change if we want to compute the velocities on the fly (thus avoiding allocations).

I changed a bit the API, namely:

  • Every time a parameter assumes a specific measure unit, the name of the keyword states it accordingly (e.g., vv_km_s if the velocity must be in km/s)
  • I remove the dipoleunits parameter, as at the moment it is not used (it's easy to put it back)
  • I changed the order of a few parameters in the functions to make sure that those parameters which are likely to be always set to some value will permit to specify a default.
  • I changed the way the dipole formula is specified: instead of using strings (handy, but computationally inefficient when used within for loops), I used IntEnum and moved a few ifs outside of the for loops.

What's missing are tests and documentation.

@mreineck
Copy link
Collaborator

mreineck commented Sep 5, 2021

I have no experience with numba so far, so maybe this is a stupid question about the dipole calculation: does the level of approximation really make a significant difference as far as performance is concerned?
If this is, say, a ten percent effect, I'd propose to go for code simplification and just implement the exact dipole contribution.

@ziotom78
Copy link
Member Author

ziotom78 commented Sep 6, 2021

I have no experience with numba so far, so maybe this is a stupid question about the dipole calculation: does the level of approximation really make a significant difference as far as performance is concerned?
If this is, say, a ten percent effect, I'd propose to go for code simplification and just implement the exact dipole contribution.

Hi Martin, I'll give my answer and then let @paganol to comment.

I agree that there should be no appreciable difference in the running time of the many implementations of the dipole estimator. However, if we want to do comparisons with results produced by other existing codes, there are chances that those codes use different conventions to compute the dipole. Having a reasonable set of models implemented in our framework could help people in doing meaningful comparisons with other estimates.

Moreover, it might be interesting to compare the estimates between a rough model and a more refined model. If we discover that some correction is negligible, then this might be a hint for theoreticians working on LiteBIRD that it's ok to use a more simplified formula in their calculations.

@paganol
Copy link
Member

paganol commented Sep 8, 2021

Hi @mreineck, I agree with @ziotom78's comment. Different approximations have probably similar running time. I think the reason for having them implemented is in the possibility of testing the impact of higher-order corrections, like frequency-dependent kinematic quadrupole. For example, those terms might have an impact on calibration and, with this implementation, we can test it.

@mreineck
Copy link
Collaborator

mreineck commented Sep 9, 2021

Thanks a lot for the explanations!

@ziotom78 ziotom78 marked this pull request as ready for review September 9, 2021 15:17
@ziotom78 ziotom78 merged commit e0ba5a9 into master Sep 17, 2021
@ziotom78 ziotom78 deleted the dipole branch September 17, 2021 19:47
@paganol paganol mentioned this pull request Sep 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants