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

consolidate user_add_params #105

Merged
merged 1 commit into from
Dec 6, 2024
Merged

Conversation

yichengt900
Copy link
Collaborator

@yichengt900 yichengt900 commented Dec 5, 2024

This PR addresses issue #104 and includes the following updates:

  1. Cleanup: Removed unnecessary module imports from cobalt_param_doc.F90.
  2. Duplicate Removal: Eliminated the redundant user_add_params call from generic_COBALT_init.
  3. Code Refinement: Consolidated all get_param calls within the user_add_params subroutine to enhance its robustness and maintainability.

These changes have been validated through 1D regression testing as well as NWA12, NEP10, and OM4p25 COBALT regression tests, confirming no impact on baseline results. However, due to the reorganization of get_param calls, the parameter order in COBALT_parameter_doc.* has been slightly modified. Importantly, the parameter values remain consistent and unchanged.

@yichengt900 yichengt900 self-assigned this Dec 5, 2024
@yichengt900 yichengt900 marked this pull request as ready for review December 5, 2024 18:23
Copy link
Contributor

@charliestock charliestock left a comment

Choose a reason for hiding this comment

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

Hello @yichengt900 and @theresa-morrison - this looks great to me. Forge ahead!

@theresa-morrison
Copy link
Contributor

Thanks @yichengt900! These changes look like what we discussed and it is good that they are not changing answers.

@yichengt900 yichengt900 merged commit b2326f6 into dev/cefi Dec 6, 2024
1 check passed
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.

3 participants