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

Broken hash monkeypatching in aiida-core v2.6.3 #80

Open
PhilippRue opened this issue Nov 20, 2024 · 12 comments
Open

Broken hash monkeypatching in aiida-core v2.6.3 #80

PhilippRue opened this issue Nov 20, 2024 · 12 comments
Labels
help wanted Extra attention is needed topic/export-cache Concerns the automatic cache export feature type/bug Something isn't working type/question Further information is requested

Comments

@PhilippRue
Copy link
Contributor

I want to update my tests on the aiida-kkr plugin to use the enable_archive_cache feature. Doing that I encountered a few problems:

  • When doing this I noticed that aiida-test-chache wants to have an older aiida version (aiida-core<2.3). Is there a reason for this?
  • I am currently working on aiida-core==v2.6.3 so I simply tried to relax the version constraint on aiida-core and then the installation works
  • However, when I try to use the enable_archive_cache feature, I always get a different hash for a simple calculation. At first I thought this is due to the code input that changes with each run (uses a temporary testing DB and installs a new code that then has a different hash).
  • So I managed to remove the code input from the _get_objects_to_hash list of the calculation I am doing using the .aiida-testing-config.yml file (btw. this should also be renamed to aiida-test-cache-config.yml, right?).
  • Now my objects_to_hash list of the process is identical which means I should get the same hash and caching should work. But it obviously doesn't.

This is a code snippet how I use the enable_archive_cache feature in my test:

def test_voronoi_after_kkr(aiida_profile_clean, voronoi_local_code, enable_archive_cache):
    ...
    builder = VoronoiCalculation.get_builder()
    builder.code = voronoi_local_code
    builder.metadata.options = options
    builder.parameters = new_params
    builder.parent_KKR = parent_calc_remote

    # now run calculation (or use cached results)
    with enable_archive_cache(data_dir/'voronoi_after_kkr.aiida'):
        out, node = run_get_node(builder)
    print('hash:', node.get_hash())
    print('cache_source:', node.get_cache_source())
    print('code objects to hash:', node._get_objects_to_hash())
    print('ignored attributes:', node._hash_ignored_attributes)
    ...

My .aiida-testing-config.yml looks like this (exclude the code input):

$ cat .aiida-testing-config.yml 
archive_cache:
    ignore:
        calcjob_inputs: ['code'] #List of link labels of inputs to ignore in the aiida hash

And then this is how the output about the hashing looks like for two different runs:

hash: f07a302ee8d5ff99c2893890639a498838361a06b74040ea7496515c802d6c54
cache_source: None
code objects to hash: [{'resources': {'num_machines': 1, 'tot_num_mpiprocs': 1}, 'input_filename': 'inputcard', 'output_filename': 'out_voronoi', 'submit_script_filename': '_aiidasubmit.sh', 'scheduler_stdout': '_scheduler-stdout.txt', 'scheduler_stderr': '_scheduler-stderr.txt', 'custom_scheduler_commands': '', 'mpirun_extra_params': [], 'import_sys_environment': True, 'environment_variables': {}, 'environment_variables_double_quotes': False, 'prepend_text': '', 'append_text': '', 'parser_name': 'kkr.voroparser'}, {'parameters': '5e756780f92ea2de202dded0176615ee458e4b2400f0541fd449bc239fc5965f', 'parent_KKR': None}]
ignored attributes: ('metadata_inputs', 'queue_name', 'account', 'qos', 'priority', 'max_wallclock_seconds', 'max_memory_kb', 'version', 'version')

second run (everything is the same except for the hash which then prevents caching)

hash: 9e87545b0eaf3a4ea487d2e84032c6d5e1e2bb3c8bac73812bb7cd2d88c414e7
cache_source: None
code objects to hash: [{'resources': {'num_machines': 1, 'tot_num_mpiprocs': 1}, 'input_filename': 'inputcard', 'output_filename': 'out_voronoi', 'submit_script_filename': '_aiidasubmit.sh', 'scheduler_stdout': '_scheduler-stdout.txt', 'scheduler_stderr': '_scheduler-stderr.txt', 'custom_scheduler_commands': '', 'mpirun_extra_params': [], 'import_sys_environment': True, 'environment_variables': {}, 'environment_variables_double_quotes': False, 'prepend_text': '', 'append_text': '', 'parser_name': 'kkr.voroparser'}, {'parameters': '5e756780f92ea2de202dded0176615ee458e4b2400f0541fd449bc239fc5965f', 'parent_KKR': None}]
ignored attributes: ('metadata_inputs', 'queue_name', 'account', 'qos', 'priority', 'max_wallclock_seconds', 'max_memory_kb', 'version', 'version')
@PhilippRue
Copy link
Contributor Author

@danielhollas I noticed that you have been working in this package recently. Do you have any insights about this?

@PhilippRue PhilippRue added type/bug Something isn't working help wanted Extra attention is needed topic/export-cache Concerns the automatic cache export feature type/question Further information is requested labels Nov 20, 2024
@PhilippRue
Copy link
Contributor Author

maybe this is related to #71?

PhilippRue added a commit to JuDFTteam/aiida-kkr that referenced this issue Nov 20, 2024
@PhilippRue
Copy link
Contributor Author

Update: downgrading to aiida-core==v2.2.2 fixes the hashing and thus also the caching feature

$ verdi status
 ✔ version:     AiiDA v2.2.2

Then after rerunning once with --archive-cache-overwrite I get an updated archive file that is then used to find the cache_source in a subsequent run:

hash: 8a241fc732d47e52a1ae64b4afc5d8625fb3d4f6c6bd0b4a6e8ba345ebc6691c
cache_source: f4bf1967-095f-44c6-ac5c-2ba9342892f4
code objects to hash: [{'withmpi': False, 'resources': {'num_machines': 1, 'tot_num_mpiprocs': 1}, 'append_text': '', 'parser_name': 'kkr.voroparser', 'prepend_text': '', 'input_filename': 'inputcard', 'output_filename': 'out_voronoi', 'scheduler_stderr': '_scheduler-stderr.txt', 'scheduler_stdout': '_scheduler-stdout.txt', 'mpirun_extra_params': [], 'environment_variables': {}, 'import_sys_environment': True, 'submit_script_filename': '_aiidasubmit.sh', 'custom_scheduler_commands': '', 'environment_variables_double_quotes': False}, {'parameters': '412196cae29ce353a05a7b362568f18f8f4a073ba4ddf7553284898d585d63c3', 'parent_KKR': '280e85ad5f1716b2ddd1d9c37e24cfd438dd9125684addc83f974a478bb592de'}]
ignored attributes: ('queue_name', 'account', 'qos', 'priority', 'max_wallclock_seconds', 'max_memory_kb', 'version', 'version', 'version', 'version')

@danielhollas
Copy link
Collaborator

Hi!

Looks like you picked an unfortunate time. I am currently working on this as part of AiiDA coding week, please see #74., so things are in flux.

When doing this I noticed that aiida-test-chache wants to have an older aiida version (aiida-core<2.3). Is there a reason for this?

Yes, as you found out, I added this pin temporarily until #71 is resolved. It will definitely need changes to the fixtures here. But in any case, thanks for testing, it's good to know that things are actually broken. :-)

(btw. this should also be renamed to aiida-test-cache-config.yml, right?).

Yep, I renamed this package literally yesterday and this is a follow up that I'll most likelly do today.

It's great to see these fixtures being used, I was not sure if I should invest my time in this since I didn't found a lot of users on public GitHub repositories. Is aiida-kkr public?

Thanks for the report!

@danielhollas
Copy link
Collaborator

@PhilippRue actually I have one question. Could you try your tests with AiiDA 2.3?

@PhilippRue
Copy link
Contributor Author

Hi, yes aiida-kkr is public. But so far I used an old forked version from aiida-testing. So it is time to make the switch ;)

I made some more tests and it breaks with the change from 2.5 to 2.6:

  • AiiDA v2.3.1 works
  • AiiDA v2.4.3 works
  • AiiDA v2.5.2 works
  • AiiDA v2.6.0 broken
  • AiiDA v2.6.3 broken

@PhilippRue
Copy link
Contributor Author

FYI: I just forked this repo to have a temporary version with a different aiida-core version constraint (<2.6). I'll use that in the meantime to test if my tests in aiida-kkr now work again. I'll then switch back to this one when the development converges.

And thanks a lot @danielhollas for updating this package!

@danielhollas
Copy link
Collaborator

@PhilippRue cheers! By the way, are you using the mock_code fixture as well, or only archive_cache?

What is currently blocking me is the failed mock_code test for aiida 2.3 Once that is resolved, I am planning to release a version of this package to PyPI on which you could then depend on.

@PhilippRue
Copy link
Contributor Author

@PhilippRue cheers! By the way, are you using the mock_code fixture as well, or only archive_cache?

I only use archive_cache

What is currently blocking me is the failed mock_code test for aiida 2.3 Once that is resolved, I am planning to release a version of this package to PyPI on which you could then depend on.

I'm looking forward to that ;)

@danielhollas
Copy link
Collaborator

What is currently blocking me is the failed mock_code test for aiida 2.3 Once that is resolved, I am planning to release a version of this package to PyPI on which you could then depend on.

This should be solved in #84. Once merged we can make a release that supports aiida 2.5 (but not 2.6 yet). Hopefully next week.

@danielhollas
Copy link
Collaborator

FYI: I just forked this repo to have a temporary version with a different aiida-core version constraint (<2.6). I'll use that in the meantime to test if my tests in aiida-kkr now work again. I'll then switch back to this one when the development converges.

Hi @PhilippRue, I've just published a first version (0.0.1) of this package on PyPI https://pypi.org/project/aiida-test-cache/. It would be great if you could test it out. It supports AiiDA versions up to v2.5.

I've made some progress on supporting v2.6 (see #85) but it's not quite ready yet.

@PhilippRue
Copy link
Contributor Author

Hi @PhilippRue, I've just published a first version (0.0.1) of this package on PyPI https://pypi.org/project/aiida-test-cache/. It would be great if you could test it out. It supports AiiDA versions up to v2.5.

Works like a charm with enable_archive_cache. Thanks a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed topic/export-cache Concerns the automatic cache export feature type/bug Something isn't working type/question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants