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

Units issue in drag equations #297

Closed
jake-asquith-spire opened this issue Apr 25, 2024 · 3 comments
Closed

Units issue in drag equations #297

jake-asquith-spire opened this issue Apr 25, 2024 · 3 comments
Labels

Comments

@jake-asquith-spire
Copy link
Contributor

Hello there,

Is there a units problem in the equations for calculating drag force (e.g. code)?

From looking at other force calculations in the repo that use the same ForceModel trait, I can see that they are returning 10^-3 * Newtons. E.g. for solar pressure it looks like the equation has (10^-3 * m^2 * N * m^-2)

Similarly for the gravitational acceleration if we multiply by mass (I suppose kg, as that seems conventional to other parts of the repo), we derive the units (kg * km * s^-2) => (kg * 10^-3 * m * s^-2) => (10^-3 * N).

For the aforementioned drag equation though, it looks like the units of the equation in existing code are (kg * m^-3 * m^2 * km^2 * s^-2), which resolves to ((kg * km^2 * s^-2) / m^1), which doesn't equal the others. It seems like it might be appropriate to multiply the equation out by 10^3 here, which would give ((kg * km^2 * s^-2) / (m^1 * 10^-3)) => (kg * km * s^-2) => (10^-3 * N), which is consistent with the other forces.

Thoughts? Happy to open a PR to change this.

@ChristopherRabotin
Copy link
Member

Hi Jake,

I think you're absolutely correct. The drag models have not been validated as the other models have, in part because I have worked only on cislunar missions for 7-8 years now, but also because these models are much simpler than the ones I could find in other trustworthy software. If you have a verification case you could add, that would be even better, but if you don't , I totally understand.

Also note that #86 will significantly improve the gravity models, and I'm actively working on this (although https://github.com/nyx-space/hifitime/milestone/14 has been taking a lot of my free time recently).

Thanks

@jake-asquith-spire
Copy link
Contributor Author

No problem!

Shall I open a PR to put in the missing factors, or will the units change be encompassed in #86 (I see some mention about drag changes in there)?

Thanks for linking that milestone, I had a look at the nyx-space ones too.

@ChristopherRabotin
Copy link
Member

Yes, please open a PR with the unit changes. I expect that you'll need to update the associated regression test. Ticket #86 still needs quite some work, so I'll be sure to update that branch to whatever is on master before merging. That branch adds a lot of breaking changes though, and I'll be happy to guide you through updating your code base once it's merged.

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

No branches or pull requests

2 participants