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

Rename phase transition index #6169

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lhy11009
Copy link
Contributor

@lhy11009 lhy11009 commented Dec 1, 2024

Pull Request Checklist. Please read and check each box with an X. Delete any part not applicable. Ask on the forum if you need help with any step.

Renamed the variable phase_index to phase_transition_index across
affected functions and structures. The new variable name, phase_transition_index,
provides a more precise and descriptive representation of its role,
emphasizing its purpose in indexing phase transitions rather than phases.

Before your first pull request:

For all pull requests:

For new features/models or changes of existing features:

  • I have tested my new feature locally to ensure it is correct.
  • I have added a changelog entry in the doc/modules/changes directory that will inform other users of my change.

@lhy11009
Copy link
Contributor Author

lhy11009 commented Dec 2, 2024

Some fact check before progress

  • In the tompgraphy_based_plate_motions (both .h and .cc files), the "phase_index" means phase, so no change is applied.
  • The diffusion_viscosity (and other similar functions of rheology) in grain_size (both .h and .cc files) means phase
  • In the include/aspect/material_model/utilities.h and .cc files
  1. All the in.phase_index are changed to in.phase_transition_index
  2. All the phase_inputs.phase_index are changed to phase_inputs.phase_transition_index
  3. In the member functions of PhaseFunction, all the phase_index are changed to phase_transition_index.
  4. In phase_average_value, it means phase.
  • In the visco_plastic.cc file
  1. All the in.phase_index are changed to in.phase_transition_index
  2. All the phase_inputs.phase_index are changed to phase_inputs.phase_transition_index

@lhy11009 lhy11009 force-pushed the rename_phase_transition_index branch from 728d7dc to 1156666 Compare December 2, 2024 00:33
Copy link
Member

@gassmoeller gassmoeller left a comment

Choose a reason for hiding this comment

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

Very nice, I have a suggestion for shortening the changelog entry, and please squash the two commits into one (or give the second one a better commit message like 'Add changelog').

Afterwards this is good to go.

doc/modules/changes/20241201_lhy11009 Outdated Show resolved Hide resolved
@gassmoeller
Copy link
Member

/rebuild

@lhy11009 lhy11009 force-pushed the rename_phase_transition_index branch from 33c2f97 to ea5118f Compare December 2, 2024 16:05
@lhy11009
Copy link
Contributor Author

lhy11009 commented Dec 2, 2024

@gassmoeller Thanks for your review, I have committed the change and merged the commits.

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

Successfully merging this pull request may close these issues.

2 participants