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

Quick fix for 'KeyError: _manual' bug caused by unmet requirements. #275

Merged
merged 3 commits into from
Dec 2, 2022

Conversation

HTJense
Copy link
Contributor

@HTJense HTJense commented Dec 1, 2022

In some instances, whenever requirements are unmet (e.g. a theory/likelihood code relies on a parameter that is neither sampled nor supplied), cobaya attempts to raise a warning containing the info about said unmet requirement. The code right before that raise statement, however, can sometimes fault itself, because Cobaya tries to remove the unmet requirement from the manual input list, which is not necessarily initialized at this point. This causes an error with the cryptic error message:

---------------------------------------------------------------------------
KeyError                                  Traceback (most recent call last)
/tmp/ipykernel_69652/2871735423.py in <module>
     47 }
     48 
---> 49 model = get_model(model_dict)

~/.local/lib/python3.10/site-packages/cobaya/model.py in get_model(info_or_yaml_or_file, debug, stop_at_error, packages_path, override)
   1351             yaml_dump(sort_cosmetic(updated_info)))
   1352     # Initialize the parameters and posterior
-> 1353     return Model(updated_info["params"], updated_info["likelihood"],
   1354                  updated_info.get("prior"), updated_info.get("theory"),
   1355                  packages_path=info.get(packages_path_input),

~/.local/lib/python3.10/site-packages/cobaya/model.py in __init__(self, info_params, info_likelihood, info_prior, info_theory, packages_path, timing, allow_renames, stop_at_error, post, skip_unused_theories, dropped_theory_params)
    242         # Assign input/output parameters
    243         self._assign_params(info_likelihood, info_theory, dropped_theory_params)
--> 244         self._set_dependencies_and_providers(skip_unused_theories=skip_unused_theories)
    245         # Add to the updated info some values that are only available after initialisation
    246         self._updated_info = recursive_update(

~/.local/lib/python3.10/site-packages/cobaya/model.py in _set_dependencies_and_providers(self, manual_requirements, skip_unused_theories)
    761                         print(requirements)
    762                         requirements[manual_theory] = [
--> 763                             req for req in requirements[manual_theory]
    764                             if req.name != requirement.name]
    765                         raise LoggedError(self.log,

KeyError: _manual

A simple fix for this, provided here, replaces the requirements[manual_theory] lookup with requirements.get(manual_theory, []), to avoid the error.

@HTJense
Copy link
Contributor Author

HTJense commented Dec 1, 2022

An alternative fix for this would be to replace the bugged line with if manual_theory in requirements: requirements[manual_theory] = [ ... ] or similar.

@cmbant
Copy link
Collaborator

cmbant commented Dec 1, 2022

Thanks, this looks fine to me. I tried to fix the flake test error on master, hopefully will pass tests after merge.

@codecov-commenter
Copy link

codecov-commenter commented Dec 1, 2022

Codecov Report

Merging #275 (bf41cf1) into master (48c6391) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #275   +/-   ##
=======================================
  Coverage   87.61%   87.61%           
=======================================
  Files          92       92           
  Lines        8380     8380           
=======================================
  Hits         7342     7342           
  Misses       1038     1038           
Impacted Files Coverage Δ
cobaya/model.py 93.89% <ø> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@cmbant cmbant merged commit f2a90f9 into CobayaSampler:master Dec 2, 2022
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

3 participants