Skip to content

Refactoring#53

Merged
RemDelaporteMathurin merged 10 commits into
mainfrom
rem/refactoring
Nov 29, 2024
Merged

Refactoring#53
RemDelaporteMathurin merged 10 commits into
mainfrom
rem/refactoring

Conversation

@RemDelaporteMathurin
Copy link
Copy Markdown
Collaborator

@RemDelaporteMathurin RemDelaporteMathurin commented Nov 25, 2024

This PR provides refactoring for the temperature and particle fluxes functions.

I did not provide tests at this stage and will keep it for another PR as I suspect a new class will be created for this

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 3 out of 3 changed files in this pull request and generated no suggestions.

Comments skipped due to low confidence (2)

src/hisp/festim_models/mb_model.py:758

  • [nitpick] The function name 'T_function_W_pure' is not very descriptive. Consider renaming it to 'calculate_temperature_W'.
def T_function_W_pure(x, heat_flux, coolant_temp, thickness):

src/hisp/festim_models/mb_model.py:766

  • [nitpick] The function name 'T_function_B_pure' is not very descriptive. Consider renaming it to 'calculate_temperature_B'.
def T_function_B_pure(heat_flux, coolant_temp):

Comment thread src/hisp/festim_models/mb_model.py Outdated
@RemDelaporteMathurin RemDelaporteMathurin marked this pull request as ready for review November 25, 2024 21:56
Copy link
Copy Markdown
Collaborator

@kaelyndunnell kaelyndunnell left a comment

Choose a reason for hiding this comment

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

Looks great! Note to self that will follow this same model for the ion and atom implantation ranges implementation.

Comment thread src/hisp/festim_models/mb_model.py
Comment thread src/hisp/festim_models/mb_model.py Outdated
Comment thread src/hisp/festim_models/mb_model.py Outdated
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.

4 participants