Skip to content

Fix coords in constant_data issue #5046 #5062

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

Merged
merged 2 commits into from
Oct 9, 2021

Conversation

michaelosthege
Copy link
Member

I got a minimum example to reproduce the problem.

@OriolAbril it looks like ArviZ 0.11.4 also has a bug there. This is the traceback when passing to dict_to_dataset from ArviZ:

_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _  

shape = (1, 17)
coords = {'backwards': <xarray.IndexVariable 'backwards' (backwards: 17)>
array([16, 15, 14, 13, 12, 11, 10,  9,  8,  7,  6,  5...: <xarray.IndexVariable 'draw' (draw: 17)>
array([ 0,  1,  2,  3,  4,  5,  6,  7,  8,  9, 10, 11, 12, 13, 14, 15, 16])}
dims = ['chain', 'draw', 'backwards']

    def _infer_coords_and_dims(
        shape, coords, dims
    ) -> "Tuple[Dict[Any, Variable], Tuple[Hashable, ...]]":
        """All the logic for creating a new DataArray"""

        if (
            coords is not None
            and not utils.is_dict_like(coords)
            and len(coords) != len(shape)
        ):
            raise ValueError(
                f"coords is not dict-like, but it has {len(coords)} items, "
                f"which does not match the {len(shape)} dimensions of the "
                "data"
            )

        if isinstance(dims, str):
            dims = (dims,)

        if dims is None:
            dims = [f"dim_{n}" for n in range(len(shape))]
            if coords is not None and len(coords) == len(shape):
                # try to infer dimensions from coords
                if utils.is_dict_like(coords):
                    # deprecated in GH993, removed in GH1539
                    raise ValueError(
                        "inferring DataArray dimensions from "
                        "dictionary like ``coords`` is no longer "
                        "supported. Use an explicit list of "
                        "``dims`` instead."
                    )
                for n, (dim, coord) in enumerate(zip(dims, coords)):
                    coord = as_variable(coord, name=dims[n]).to_index_variable()
                    dims[n] = coord.name
            dims = tuple(dims)
        elif len(dims) != len(shape):
>           raise ValueError(
                "different number of dimensions on data "
                f"and dims: {len(shape)} vs {len(dims)}"
            )
E           ValueError: different number of dimensions on data and dims: 2 vs 3

..\..\appdata\local\continuum\miniconda3\envs\pm3v4\lib\site-packages\xarray\core\dataarray.py:126: ValueError
================================================================================================== warnings summary ================================================================================================== 
..\..\appdata\local\continuum\miniconda3\envs\pm3v4\lib\site-packages\win32\lib\pywintypes.py:2
  c:\users\osthege\appdata\local\continuum\miniconda3\envs\pm3v4\lib\site-packages\win32\lib\pywintypes.py:2: DeprecationWarning: the imp module is deprecated in favour of importlib; see the module's documentation for alternative uses
    import imp, sys, os

pymc/tests/test_idata_conversion.py::TestDataPyMC::test_constant_data_coords_issue_5046
  c:\users\osthege\appdata\local\continuum\miniconda3\envs\pm3v4\lib\site-packages\arviz\data\base.py:130: UserWarning: In variable alpha, there are more dims (1) given than exist (0). Passed array should have shape (chain,draw, *shape)
    warnings.warn(

-- Docs: https://docs.pytest.org/en/stable/warnings.html
============================================================================================== short test summary info =============================================================================================== 
FAILED pymc/tests/test_idata_conversion.py::TestDataPyMC::test_constant_data_coords_issue_5046 - ValueError: different number of dimensions on data and dims: 2 vs 3

@michaelosthege michaelosthege added bug trace-backend Traces and ArviZ stuff labels Oct 9, 2021
@codecov
Copy link

codecov bot commented Oct 9, 2021

Codecov Report

Merging #5062 (b3daa2d) into main (a3cc81c) will decrease coverage by 20.36%.
The diff coverage is 25.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##             main    #5062       +/-   ##
===========================================
- Coverage   78.32%   57.96%   -20.37%     
===========================================
  Files         130      130               
  Lines       24430    24439        +9     
===========================================
- Hits        19135    14165     -4970     
- Misses       5295    10274     +4979     
Impacted Files Coverage Δ
pymc/tests/test_idata_conversion.py 0.00% <0.00%> (-96.21%) ⬇️
pymc/backends/arviz.py 75.09% <100.00%> (-14.34%) ⬇️
pymc/tests/test_ode.py 0.00% <0.00%> (-100.00%) ⬇️
pymc/tests/test_profile.py 0.00% <0.00%> (-100.00%) ⬇️
pymc/tests/test_model_func.py 0.00% <0.00%> (-100.00%) ⬇️
pymc/tests/test_posdef_sym.py 0.00% <0.00%> (-100.00%) ⬇️
pymc/tests/test_model_graph.py 0.00% <0.00%> (-100.00%) ⬇️
pymc/tests/test_model.py 0.00% <0.00%> (-99.12%) ⬇️
pymc/tests/test_shape_handling.py 0.00% <0.00%> (-96.03%) ⬇️
... and 30 more

@michaelosthege
Copy link
Member Author

This bug was a nightmare. The workarounds in my plotting code were fragile and the debugging took ages. Too much staring at the bug without seeing it.

@OriolAbril @twiecki it would be awesome if we can fast-track this fix - it's blocking me quite severely.

@michaelosthege michaelosthege changed the title Add test case for constant_data coords issue #5046 Fix coords in constant_data issue #5046 Oct 9, 2021
@michaelosthege michaelosthege self-assigned this Oct 9, 2021
@michaelosthege michaelosthege added this to the v4.0.0-beta1 (vNext) milestone Oct 9, 2021
@michaelosthege
Copy link
Member Author

This red test is a flaky NUTS linear algebra tests (#5063). It has nothing to do with the changes from this PR, so I'm going to merge anyway.

@michaelosthege michaelosthege merged commit c06c2f4 into pymc-devs:main Oct 9, 2021
@michaelosthege michaelosthege deleted the issue-5046 branch October 9, 2021 11:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug trace-backend Traces and ArviZ stuff
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants