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

GAPs integration #168

Merged
merged 36 commits into from
Oct 12, 2023
Merged

GAPs integration #168

merged 36 commits into from
Oct 12, 2023

Conversation

ppinchuk
Copy link
Collaborator

@ppinchuk ppinchuk commented Sep 27, 2023

Remove reliance on gaps.legacy by updating the code to use GAPs classes/functions directly. Incidentally, this allows the removal of the reV dependency as well.

A cute bonus is that sup3r config files now can be JSON, JSON5, YAML, or TOML

Also updated references to eagle with kestrel.

Also updated reqs to use latest version of rex, which fixes the double logging issue. This had already been pushed to main previously

ppinchuk and others added 30 commits August 26, 2023 15:22
Conflicts:
	requirements.txt
	sup3r/batch/batch.py
	sup3r/bias/bias_calc.py
	sup3r/pipeline/__init__.py
	sup3r/pipeline/config.py
	sup3r/pipeline/forward_pass.py
	sup3r/pipeline/pipeline.py
	sup3r/postprocessing/collection.py
	sup3r/preprocessing/data_handling.py
	sup3r/qa/qa.py
	sup3r/qa/stats.py
	sup3r/qa/visual_qa.py
	sup3r/solar/solar.py
	sup3r/utilities/cli.py
	sup3r/utilities/regridder.py
Conflicts:
	sup3r/preprocessing/data_handling/base.py
	sup3r/utilities/regridder.py
@ppinchuk ppinchuk marked this pull request as ready for review October 5, 2023 17:39
@@ -63,7 +63,7 @@ def from_config(ctx, config_file, verbose):
cmd_log = '\n\t'.join(cmd.split('\n'))
logger.debug(f'Running command:\n\t{cmd_log}')

if hardware_option.lower() in ('eagle', 'slurm'):
if hardware_option.lower() in ('kestrel', 'eagle', 'slurm'):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Lets make ('kestrel', 'eagle', 'slurm') a global AVAILABLE_HARDWARE_OPTIONS, since it shows up so much.

@@ -23,7 +22,6 @@
'numpy': np.__version__,
'nrel-phygnn': phygnn.__version__,
'nrel-rex': rex.__version__,
Copy link
Collaborator

Choose a reason for hiding this comment

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

no more rev. yay!

sup3r/cli.py Outdated
@@ -116,16 +118,16 @@ def forward_pass(ctx, verbose):
"execution_control": {
"option": "local"
},
"execution_control_eagle": {
"option": "eagle",
"execution_control_kestrel": {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think we need this entry actually, unless this is a gaps thing? execution_control is where we put non local options also.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not a GAPs thing. All args should go in the execution_control block for sure. I kept it around since you guys used it in your documentation for eagle args. Do you want me to replace the original execution_control block with the HPC required args? Or just delete it entrierly?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah I'm not sure why we had that lol. The former would be great, thanks.

@ppinchuk ppinchuk requested a review from bnb32 October 5, 2023 19:53
@ppinchuk ppinchuk merged commit 4330268 into main Oct 12, 2023
8 checks passed
@ppinchuk ppinchuk deleted the pp/gaps_integration branch October 12, 2023 16:02
github-actions bot pushed a commit that referenced this pull request Oct 12, 2023
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.

None yet

2 participants