Skip to content

[newchem-cpp] cleanup grackle::impl::gauss_seidel::update_fields_from_tmpdens#434

Open
mabruzzo wants to merge 8 commits into
grackle-project:newchem-cppfrom
mabruzzo:ncc/cleanup_step_rate_gauss_seidel
Open

[newchem-cpp] cleanup grackle::impl::gauss_seidel::update_fields_from_tmpdens#434
mabruzzo wants to merge 8 commits into
grackle-project:newchem-cppfrom
mabruzzo:ncc/cleanup_step_rate_gauss_seidel

Conversation

@mabruzzo

@mabruzzo mabruzzo commented Oct 2, 2025

Copy link
Copy Markdown
Collaborator

To be reviewed after #425 has been merged


Basic Context

When I transcribed step_rate_g in PR #425, I split the logic between 2 functions:

  1. grackle::impl::chemistry::species_density_updates_gauss_seidel (not touched in this PR)
  2. grackle::impl::update_fields_from_tmpdens_gauss_seidel

This PR cleans up the implementation of the latter function

Changes

This PR made the following changes:

  • rename grackle::impl::update_fields_from_tmpdens_gauss_seidel so that it's now called grackle::impl::gauss_seidel::update_fields_from_tmpdens
  • factor out a bunch of logic into helper functions that live in the grackle::impl::gauss_seidel namespace
  • started using clang-format to format step_rate_gauss_seidel.hpp

@mabruzzo mabruzzo added the refactor internal reorganization or code simplification with no behavior changes label Oct 2, 2025
@brittonsmith brittonsmith changed the base branch from newchem-cpp to main January 13, 2026 14:26
@brittonsmith brittonsmith changed the base branch from main to newchem-cpp January 13, 2026 14:26
@brittonsmith

Copy link
Copy Markdown
Contributor

This is producing fairly large differences in the metal-chemistry freefall tests. I think we need to take a closer look. I'm going to go back and make sure nothing has deviated from the last couple PRs I've merged.

@mabruzzo

Copy link
Copy Markdown
Collaborator Author

The fact that an earlier commit was failing may indicate that there is a bug in this PR. It may make sense to punt on this for now

@brittonsmith

Copy link
Copy Markdown
Contributor

Ok, that sounds sensible. I bypass this one for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactor internal reorganization or code simplification with no behavior changes

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

2 participants