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

Introduce ODProblem #87

Merged
merged 17 commits into from
Oct 11, 2024
Merged

Introduce ODProblem #87

merged 17 commits into from
Oct 11, 2024

Conversation

LuEdRaMo
Copy link
Collaborator

@LuEdRaMo LuEdRaMo commented Oct 1, 2024

This PR improves the orbit determination interface by introducing ODProblem. ODProblem is meant to represent an orbit determination problem, applicable but not limited to Near Earth Objects, consisting of:

  1. a dynamical model,
  2. a vector of (optical) observations,
  3. a weighting scheme and,
  4. a debiasing scheme.

This new struct will give the user a complete control over the astrometry error model and help with the internal organization of the package.

@LuEdRaMo
Copy link
Collaborator Author

LuEdRaMo commented Oct 2, 2024

Since this new interface is meant to represent an arbitrary ODProblem, I think we should rename NEOParameters to ODParameters for consistency. What do you think @lbenet @PerezHz ?

@PerezHz
Copy link
Owner

PerezHz commented Oct 6, 2024

First of all many thanks for this PR! I would say, if NEOParameters includes only parameters related to OD then yes I would be in favor, but if it also includes parameters related, say, to propagation, then maybe we could also consider grouping the fields in NEOParameters in OD, propagation and possibly other parameters (e.g., close approach?).

@LuEdRaMo
Copy link
Collaborator Author

LuEdRaMo commented Oct 6, 2024

First of all many thanks for this PR! I would say, if NEOParameters includes only parameters related to OD then yes I would be in favor, but if it also includes parameters related, say, to propagation, then maybe we could also consider grouping the fields in NEOParameters in OD, propagation and possibly other parameters (e.g., close approach?).

Currently, NEOParameters contains all the relevant parameters used in the OD interface. Such parameters can be grouped in different categories: propagation, Gauss method, too short arc, least squares and outlier rejection; but all are part of the OD pipeline. I think it might be cumbersome to have a different struct per category...

@LuEdRaMo LuEdRaMo marked this pull request as ready for review October 9, 2024 19:09
@LuEdRaMo
Copy link
Collaborator Author

This is ready and tests pass, so I'll proceed to merge to continue with the next PRs.

Since this new interface is meant to represent an arbitrary ODProblem, I think we should rename NEOParameters to ODParameters for consistency. What do you think @lbenet @PerezHz ?

I'll probably open an issue to discuss this more thoroughly, as NEOs.jl is becoming a sort of OrbitDetermination.jl.

@LuEdRaMo LuEdRaMo merged commit ab8c441 into main Oct 11, 2024
4 of 6 checks passed
@PerezHz
Copy link
Owner

PerezHz commented Oct 14, 2024

Currently, NEOParameters contains all the relevant parameters used in the OD interface. Such parameters can be grouped in different categories: propagation, Gauss method, too short arc, least squares and outlier rejection; but all are part of the OD pipeline. I think it might be cumbersome to have a different struct per category...

I agree it would be cumbersome to have a different struct for each of the categories you mention and just to be clear my suggestion was not going in that direction. Rather, I would say there are two main categories here: orbit determination and orbit propagation. If we think about an user who has some initial conditions and wants to propagate an orbit, say, for long-term impact monitoring applications does not necessarily need orbit determination for that. Such user would probably initialize an instance of a NEOParameters without giving much thought to OD parameters. In that sense I meant NEOParameters includes things beyond "only" orbit determination.

@PerezHz
Copy link
Owner

PerezHz commented Oct 14, 2024

I'll probably open an issue to discuss this more thoroughly, as NEOs.jl is becoming a sort of OrbitDetermination.jl.

It's an interesting idea and I agree we should discuss this. One argument against changing the name of this repo is that there is already published research linking here, and while we can document any and all changes pointing people to the right place, I think it's just easier to leave it like this. For the time being I'd incline myself more towards maybe re-organizing this repo in submodules, for example, one dealing with MPC-related functionality; another dealing with astrometry observation (and error) models; another one with the core orbit determination functionality, etc. It's probably worth opening an issue and continue the discussion there.

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.

2 participants