Skip to content

Conversation

@The9Cat
Copy link
Member

@The9Cat The9Cat commented Jan 21, 2026

Includes

  • a LaTeX write up
  • PDF of the same
  • Python script for generating conversions
  • README.md explaining the contents and examples
  • Example files

Movitation

The EXP drop in on Jan 21, 2026.

Comments

  • I did try to check all of this, but there may be mistakes...please help check and fix!!
  • I'm not sure that this helps clarify or makes the confusion worse. LMK what you think.
  • I hardwired a Gadget and Bonsai unit example in the script but I don't think I have the same units that Jason actually uses, so that would be nice to fix. 
  • Also, if we decide to keep this PR, it would be easy to put in other premade unit stanzas to cover other options.  Are there any other that we should put in?

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request adds comprehensive documentation and tooling for unit conversions in systems where the gravitational constant G is set to 1, specifically for the EXP simulation code. The PR includes mathematical derivations, a Python conversion script, example data files, and usage documentation.

Changes:

  • LaTeX document with mathematical derivations for unit conversion formulas
  • Python script for computing scale factors and converting particle files
  • Example Earth-Sun system files demonstrating the conversion process

Reviewed changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
EXP-units/expunits.tex LaTeX documentation explaining unit conversion mathematics and formulas
EXP-units/expunits.py Python script implementing scale factor calculations and particle file conversion
EXP-units/earth_sun_scaled.txt Example output file showing Earth-Sun system in scaled units
EXP-units/earth_sun_cgs.txt Example input file with Earth-Sun system in CGS units
EXP-units/README_expunits.md Documentation explaining the contents and providing usage examples

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 117 to 118
if abs((G_check - G_new) / (abs(G_new) if G_new != 0 else 1.0)) > 1e-9:
raise ValueError(f"Inconsistent scales: G_new requested {G_new} but scales give {G_check}.")
Copy link

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The condition "abs((G_check - G_new) / (abs(G_new) if G_new != 0 else 1.0)) > 1e-9" will use the denominator 1.0 when G_new is 0, but G_new=0 is not a physically meaningful value for a gravitational constant. Consider raising a more informative error if G_new is 0 or negative before this check.

Copilot uses AI. Check for mistakes.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot open a new pull request to apply changes based on this feedback

The9Cat and others added 2 commits January 21, 2026 16:46
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link

Copilot AI commented Jan 21, 2026

@The9Cat I've opened a new pull request, #12, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot AI and others added 7 commits January 21, 2026 21:50
Co-authored-by: The9Cat <25960766+The9Cat@users.noreply.github.com>
Co-authored-by: The9Cat <25960766+The9Cat@users.noreply.github.com>
Co-authored-by: The9Cat <25960766+The9Cat@users.noreply.github.com>
Co-authored-by: The9Cat <25960766+The9Cat@users.noreply.github.com>
Add input validation for gravitational constants in compute_scales
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 7 changed files in this pull request and generated 11 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

The9Cat and others added 4 commits January 21, 2026 18:43
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link

Copilot AI commented Jan 21, 2026

@The9Cat I've opened a new pull request, #13, to work on those changes. Once the pull request is ready, I'll request review from you.

The9Cat and others added 3 commits January 21, 2026 18:44
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copilot AI and others added 18 commits January 21, 2026 23:47
Co-authored-by: The9Cat <25960766+The9Cat@users.noreply.github.com>
Add Motivation section to EXP-units README
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Added details about phase-space conversion and provided a Python script for unit conversion.
Updated README to reflect focus on EXP unit conversion for G=1 and clarified example context.
Updated example commands and explanations in README.md for clarity.
Updated section headers and clarified output descriptions.
Updated example usage and detailed explanation of flags in README.
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