Broken hash monkeypatching in `aiida-core v2.6.3`

PhilippRue commented 3 days ago

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 commented 3 days ago

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

PhilippRue commented 3 days ago

maybe this is related to #71?

PhilippRue commented 3 days ago

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 commented 3 days ago

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 commented 3 days ago

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

PhilippRue commented 3 days ago

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 commented 2 days ago

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 commented 2 days ago

@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 commented 2 days ago

@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 ;)

aiidateam / aiida-test-cache

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