Skip to content

Conversation

@Yrisch
Copy link
Collaborator

@Yrisch Yrisch commented Jan 15, 2026

Description:
This PR fix the MASE test done in #774. It now follows what was actually done in NDSPMHD. It also adds a new routine in test_gravity.f90 that tests relative errors and performance of SFMM and FMM compared to direct sum on a plummer sphere with N particles varying between 10³ and 10⁶. A switch to toggle SFMM and FMM has been added for this purpose. However, the option is not shown in the infiles to stay with SFMM as the default.

Components modified:

  • Setup (src/setup)
  • Main code (src/main)
  • Moddump utilities (src/utils/moddump)
  • Analysis utilities (src/utils/analysis)
  • Test suite (src/tests)
  • Documentation (docs/)
  • Build/CI (build/ or github actions)

Type of change:

  • Bug fix
  • Physics improvements
  • Better initial conditions
  • Performance improvements
  • Documentation update
  • Better testing
  • Code cleanup / refactor
  • Other (please describe)

Testing:
Changed the mase tolerance. No new physics...

Did you run the bots? no

Did you update relevant documentation in the docs directory? no

Did you add comments such that the purpose of the code is understandable? yes

@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @Yrisch, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly enhances the robustness and analytical capabilities of the gravity solver tests. It addresses an existing bug in the MASE test by refining its calculation and tightening its tolerance, ensuring more accurate and stringent evaluations. Furthermore, it introduces a sophisticated new testing framework that allows for detailed performance and precision comparisons between SFMM, FMM, and direct summation across a range of particle numbers and accuracy settings, providing valuable data for understanding the methods' characteristics. The changes also include internal mechanisms for controlling the gravity solver and random number generation, primarily to support these new testing features.

Highlights

  • MASE Test Fix and Tolerance Adjustment: The Mean Absolute Scaled Error (MASE) test, originally from PR (testgrav) added unit test for acceleration in Plummer sphere #774, has been corrected to align with the methodology used in NDSPMHD. The calculation of MASE has been revised, and the tolerance for this test has been significantly tightened from 1.2e-1 to 8.e-4, leading to a much stricter evaluation criterion.
  • SFMM/FMM Switch for Testing: A new neigh_switch integer variable has been introduced in neigh_kdtree.f90 to allow toggling between Smoothed Fast Multipole Method (SFMM) and Fast Multipole Method (FMM) implementations. While this switch is configurable internally for testing purposes, it is intentionally not exposed in the input files to maintain SFMM as the default method for users.
  • Centralized Random Seed for Monte Carlo Particle Generation: The random seed for Monte Carlo particle placement in set_sphere.f90 has been centralized into a public module variable iseed_mc. This allows for reproducible and controlled random number generation across multiple test realisations, improving the reliability of statistical tests.
  • New Performance and Precision Plotting Routine: A comprehensive new routine (plot_SFMM and get_plummer_prec_perf) has been added to test_gravity.f90. This routine systematically evaluates the relative errors and performance of SFMM and FMM compared to direct summation on a Plummer sphere. It tests varying particle counts (from 10^3 to 10^6) and different tree accuracy parameters, outputting data suitable for plotting performance and precision curves.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request addresses a MASE test fix and introduces a new routine to evaluate the precision and performance of SFMM and FMM against direct summation on a Plummer sphere. The changes involve refactoring the neighbor-finding logic to use a neigh_switch variable, making the random seed for Monte Carlo simulations configurable, and updating the MASE calculation. The new plot_SFMM and get_plummer_prec_perf subroutines provide valuable testing capabilities for the gravity solver.

Comment on lines 1015 to 1017
fxyz_dir = fxyzu(1:3,1:npart)
fxyz_dir = fxyzu(1:3,1:npart)
fxyz_dir = fxyzu(1:3,1:npart)
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The assignment fxyz_dir = fxyzu(1:3,1:npart) is repeated three times consecutively. This is redundant and can be simplified to a single assignment to improve efficiency and readability.

fxyz_dir = fxyzu(1:3,1:npart)

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.

1 participant