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

Feature/analyze all #88

Merged
merged 3 commits into from
Feb 25, 2025
Merged

Feature/analyze all #88

merged 3 commits into from
Feb 25, 2025

Conversation

FariborzDaneshvar-NOAA
Copy link
Collaborator

  1. Added a flag of full_training_set to analyze_ensemble.py. If:
  • TRUE: It will use all samples for training and validation
  • FALSE: It will divide samples 70/30 for training/validation.
  1. Increased the time limit of schism.sbatch and post.sbatch to be able to run large ensembles (i.e. 19) with wwm.

@FariborzDaneshvar-NOAA
Copy link
Collaborator Author

@SorooshMani-NOAA Let me know if you approve this PR, Thanks.

@SorooshMani-NOAA
Copy link
Collaborator

SorooshMani-NOAA commented Feb 24, 2025

@FariborzDaneshvar-NOAA by default do we want to set it to full?

Also do we want to have control over which runs run with full and which with 30/70 split? If we want to have this control from the input yaml then we have to make some more changes, but if we want the default to run analysis by full (no split) and only someone who knows the code should be able to switch split on the split then that's OK.

One more question ... is it meaningful to have validation if we train on all? Maybe we should just disable validation if all is selected ... it also saves some computation (not by much though) ... but most of all I think it's meaningless to have it (misleading)

@FariborzDaneshvar-NOAA
Copy link
Collaborator Author

@SorooshMani-NOAA Thanks for the comment. I added a flag validate_surrogate_model to turn ON (split 70/30, make validation plots, ...) or OFF (use all for training and skip validation part).
I tested it on Hercules and worked fine. Let me know your thoughts. Thanks

@SorooshMani-NOAA
Copy link
Collaborator

Thanks @FariborzDaneshvar-NOAA please go ahead and merge ... later we can make propagate this from the input file in a separate merge.

@FariborzDaneshvar-NOAA FariborzDaneshvar-NOAA merged commit 20bc2aa into main Feb 25, 2025
3 checks passed
@FariborzDaneshvar-NOAA FariborzDaneshvar-NOAA deleted the feature/analyze_all branch February 25, 2025 15:41
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