Skip to content

Non thermal models clean up and refactor - #295

Open
samaloney wants to merge 1 commit into
sunpy:mainfrom
samaloney:non-thermal-models-refactor
Open

Non thermal models clean up and refactor#295
samaloney wants to merge 1 commit into
sunpy:mainfrom
samaloney:non-thermal-models-refactor

Conversation

@samaloney

Copy link
Copy Markdown
Member

PR Description

Less code, more readable code, and some performance improvements

  • Remove some unused and unnecessary thin wrappers
  • The old gauss_legendre port and the fixed_quad_batch are almost identical which should now be clearer the later is numerically better as the scaling operations are not compounded in the integration as they are done at the end - changed default to fixed_quad_batch
  • Figure out source of some of the numeric constants and added comment with units
  • Pull out a bunch of static constants to module level vars
  • Modernised some of the code structure and eliminated unnecessary checks
  • More / improved comments
  • More edge case tests
  • ThickTarget ~ 1.5x and ThinTarget ~2x faster
  • Still importing constants from legacy should we just switch to the astropy values at this stage or also pull out the constant function from legacy

AI Assistance Disclosure

AI tools were used for:

  • Code generation (e.g., when writing an implementation or fixing a bug)
  • [x ] Test/benchmark generation
  • Documentation (including examples)
  • [x ] Research and understanding
  • No AI tools were used

Regardless of AI use, the human contributor remains fully responsible for correctness, design choices, licensing compatibility, and long-term maintainability.

@samaloney
samaloney force-pushed the non-thermal-models-refactor branch from 570de73 to 80c057e Compare August 4, 2026 15:23

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It much easier to see that gauss_legendre and gauss_legendre do almost the same thing the only difference is when the mapping to the real limits is done before gauss_legendre or after in gauss_legendre which mean the numerical error aren't folded in to the integration

@samaloney
samaloney force-pushed the non-thermal-models-refactor branch from 2bd53b3 to fbfc6da Compare August 5, 2026 19:55
* Increase edge case test coverage for later refactor
* Update some copy / paste errors
* Inclue in the dos
* Use public scipy API
* Modernise some of the iternals and extract constants.
@samaloney
samaloney force-pushed the non-thermal-models-refactor branch from fbfc6da to af6b2c8 Compare August 5, 2026 21:22
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