Skip to content

Include solar picontrol experiment - #164

Open
mvgarcia wants to merge 14 commits into
new_mainfrom
mvgarcia/include-solar-picontrol
Open

mvgarcia wants to merge 14 commits into
new_mainfrom
mvgarcia/include-solar-picontrol

Conversation

@mvgarcia

@mvgarcia mvgarcia commented Oct 1, 2026

Copy link
Copy Markdown
Collaborator

Refactored pi control. No unit tests yet

@atteggiani atteggiani left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you @mvgarcia for opening this PR.

The picontrol addition looks good.
I have only one comment below.

One bigger matter about this experiment (and some other similar ones that need to patch namelists) is the whole namelists patching. However, this needs proper discussion and we'll probably handle it in a dedicated meeting.

Thank you for your contributions! :)

patch_str_namelist = parser.reads(patch_str)

# Create a new namelist by patching the original namelist
pi_solar_namelist_filepath = Path("atmosphere") / "input_atm.nml"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This hardcoded information should be passed as an input argument.
Something like -O atm_namelist_filepath="{{ ATM_NAMELIST_FILEPATH }}" (defining ATM_NAMELIST_FILEPATH in variables.cylc)

And the used here like

atm_namelist_filepath = request.options["atm_namelist_filepath"]

@atteggiani atteggiani changed the title Mvgarcia/include solar picontrol Include solar picontrol experiment Oct 2, 2026
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.

2 participants