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

Add task to calculate ensemble mean and variance from background state #397

Merged
merged 12 commits into from
Aug 8, 2024

Conversation

metdyn
Copy link
Collaborator

@metdyn metdyn commented Jul 31, 2024

Description

A step towards LETKF is to calculate ensemble mean and variance:

Alternatively, this functionality may be used as a simple, independent tool.

Tested on SCU17:

swell create localensembleda  -s slurmfile.yaml -p nccs_discover_sles15

slurm_directives_tasks:
  RunJediLocalEnsembleDaExecutable:
    geos_atmosphere:
      nodes: 1
      ntasks-per-node: 36
  GetEnsembleMeanVariance:
    all:
      nodes: 1
      ntasks-per-node: 36

Dependencies

None

Impact

It is part of the localensembleda suite.

@metdyn metdyn added the enhancement New feature or request label Jul 31, 2024
@rtodling
Copy link
Contributor

This seems to work - it runs fine, but I am confused as to what's preventing the localensembleda to go beyond simply the ensemean background ... after all, this is behaving as if it were the ensemblemean suite but it's not!

@Dooruk
Copy link
Collaborator

Dooruk commented Aug 1, 2024

@rtodling and @metdyn this is missing the last commit from the develop branch.

Please see following changes. We are abandoning the static StageJedi task to be able to use relative paths and instead running StageJediCycle at each cycle, which puts static files in the {cycle_dir}:

feature/submit/ens_mean...develop

@metdyn, please notice the changes in flow.cylc for 3dvar_atmos and 3dfgat_atmos, they need to be replicated for the localensembleda suite.

@metdyn
Copy link
Collaborator Author

metdyn commented Aug 1, 2024

@rtodling and @metdyn this is missing the last commit from the develop branch.

Please see following changes. We are abandoning the static StageJedi task to be able to use relative paths and instead running StageJediCycle at each cycle, which puts static files in the {cycle_dir}:

feature/submit/ens_mean...develop

@metdyn, please notice the changes in flow.cylc for 3dvar_atmos and 3dfgat_atmos, they need to be replicated for the localensembleda suite.

Thanks, @Dooruk, I will take a look and make appropriate changes to the PR to be update with other suites and relative path.

@metdyn
Copy link
Collaborator Author

metdyn commented Aug 2, 2024

Hi @Dooruk @asewnath,

I know the ctest swell test code_tests fails at
https://github.com/GEOS-ESM/swell/blob/develop/src/swell/utilities/scripts/task_question_dicts.py#L134-L135

Error message is

FAIL: test_dictionary_comparison (swell.test.code_tests.question_dictionary_comparison_test.QuestionDictionaryTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/gpfsm/dnb07/projects/p234/yyu11/br_jedi_scu17/install_swell/src/swell/swell_ens_mean/src/swell/test/code_tests/question_dictionary_comparison_test.py", line 26, in test_dictionary_comparison
    assert tq_dicts_rc == 0
AssertionError

So I printed out the string of the dict into files:

yaml.dump(question_dict_str_in)
yaml.dump(dict_to_write_str)

diff diff in.txt out.txt

Then I print out the first two lines of differences of the two files:

7,25c7,24
<   \ are the analysis variables?\n  tasks:\n  - GenerateBClimatology\n  - PrepareAnalysis\n\
<   \  - RunJediConvertStateSoca2ciceExecutable\n  - RunJediVariationalExecutable\n\
  \  - GetEnsembleMeanVariance\n  type: string-check-list\n\nbackground_error_model:\n\

---
>   \ are the analysis variables?\n  tasks:\n  - GenerateBClimatology\n  - GetEnsembleMeanVariance\n\
>   \  - PrepareAnalysis\n  - RunJediConvertStateSoca2ciceExecutable\n  - RunJediVariationalExecutable\n\
  \  type: string-check-list\n\nbackground_error_model:\n  ask_question: false\n \

The order between GetEnsembleMeanVariance and PrepareAnalysis is swap.
So the dictionary comparison using the print out string becomes a problem when new tasks is added into task dir,
which changed glob.glob file list.
There should be a better way for the task questions yaml comparison.

Screenshot 2024-08-02 at 11 49 19 AM

Meanwhile I tried to print out the difference between the two dictionaries:

set1 = set(dict1.items())
set2 = set(dict2.items())
set1 ^ set2
(https://stackoverflow.com/questions/32815640/how-to-get-the-difference-between-two-dictionaries-in-python)
There comes an error that the dictionary is not hashable. I did not go further.

@metdyn
Copy link
Collaborator Author

metdyn commented Aug 2, 2024

I think this error means I need to regenerate tasks/task_questions.yaml, because new tasks are added in.
In this logic, then the test_dictionary_comparison function would be a good place as a generator. Or I would manually check tasks/task_questions.yaml. How was this done before?

@Dooruk
Copy link
Collaborator

Dooruk commented Aug 2, 2024

@metdyn, let me look into this branch and see what is going on. I will suggest an update to this branch afterwards as a merge. It's a simple unit test issue but I also notice some potential Stage task problems.

If you change this following line in the source code (setting $LOG_INFO to 1 in the environment didn't work for me) and build it again, tests will provide more details to show you a temporary YAML file with the proper task_questions.yaml.

# Turn off the regular info testing
os.environ["LOG_INFO"] = "0" # Set this to 1 when errors are being debugged

Sorry, this is all very obscure without documentation but this is also how I learned these things..

@metdyn
Copy link
Collaborator Author

metdyn commented Aug 2, 2024

@Dooruk thanks and that is the key! I was about to ask you why the logger.info did not work in this test code file, but logger.test() worked in another file.

metdyn and others added 4 commits August 2, 2024 18:37
* Update CODEOWNERS (#398)

* parsing ozinfo.db (#396)

Co-authored-by: Doruk Ardağ <38666458+Dooruk@users.noreply.github.com>

* task name change and yaml fixes

* oops application changes

* flow.cylc fixes, slurm directive changes

* no Datetime class without datetime (#403)

* no Datetime object without datetime

* fix wrong task questions entry

---------

Co-authored-by: Matt Thompson <matthew.thompson@nasa.gov>
Co-authored-by: Akira Sewnath <asewnath@users.noreply.github.com>
Co-authored-by: Yonggang Yu <yonggang.yu@nasa.gov>
@Dooruk
Copy link
Collaborator

Dooruk commented Aug 6, 2024

@rtodling, can you confirm RunJediEnsembleMeanVariance works on your end so we can merge this?

Copy link
Collaborator

@Dooruk Dooruk left a comment

Choose a reason for hiding this comment

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

I added a few comments. There is some duplicate functionality because RunJediEnsembleMean already handles the options where you put inside RunJediLocalDaExecutable. Simply taking them out should suffice.

Copy link
Collaborator Author

@metdyn metdyn left a comment

Choose a reason for hiding this comment

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

I created another PR to help with enHX with packets.
#405

src/swell/test/suite_tests/localensembleda-tier1.yaml Outdated Show resolved Hide resolved
Copy link
Collaborator

@Dooruk Dooruk left a comment

Choose a reason for hiding this comment

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

This is good to go on my end, let's wait on @rtodling .

Copy link
Contributor

@rtodling rtodling left a comment

Choose a reason for hiding this comment

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

This works for me - it fails in the HofX, but I understand this is another issue to be tackled elsewhere.

@Dooruk Dooruk merged commit 56514b0 into develop Aug 8, 2024
2 checks passed
@Dooruk Dooruk deleted the feature/submit/ens_mean branch August 8, 2024 17:22
@Dooruk Dooruk linked an issue Aug 14, 2024 that may be closed by this pull request
@Dooruk Dooruk mentioned this pull request Aug 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request ready for merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adding ensemble mean
3 participants