check for original coords

[TimOliverMaier]

This is my quick fix for #5760 . When coords are set manually, length_tensor in set_data has no owner. That's why it is crashing. In my fix I only check if original_coords is not None, meaning they were set explicitly (as far as I understand). However, I am pretty unsure if that's the way to go. Especially since there were a couple of issues in the last time concering coords...

[codecov]

Codecov Report

Merging #5761 (5386ae9) into main (ab593b1) will decrease coverage by 0.01%.
The diff coverage is 68.75%.

❗ Current head 5386ae9 differs from pull request most recent head a3dd519. Consider uploading reports for the commit a3dd519 to get more accurate results

@@            Coverage Diff             @@
##             main    #5761      +/-   ##
==========================================
- Coverage   89.27%   89.25%   -0.02%     
==========================================
  Files          74       74              
  Lines       13803    13808       +5     
==========================================
+ Hits        12322    12325       +3     
- Misses       1481     1483       +2     

Impacted Files	Coverage Δ
pymc/model.py	86.37% <68.75%> (-0.19%)	⬇️
pymc/sampling_jax.py	96.96% <0.00%> (ø)

[canyon289]

Add a comment to explain why this was needed, otherwise looks good

[canyon289]

Is there a way to write a test for this to ensure whatever bug is being fixed is fixed?

[michaelosthege]

In light of #5751 we'll have to do this slightly different.

Something like this:

1. if length_changed
   1. if length_tensor.owner.inputs[0].owner is None` → Warning: You're changing the shape of a shared variable in the '{dname}' dimension which was initialized from coords. Make sure to update the corresponding coords, otherwise you'll get shape issues.
   2. if not isinstance(length_belongs_to, SharedVariable) → ShapeError: You're trying to resize the '{dname}' dimension which is linked to {length_belongs_to} which can not be resized.

The i) case happens with the example from #5760.

The ii) case happens with something like this:

with pm.Model():
    a = pm.Normal("a", mu=[1,2,3], dims="fixed")
    m = pm.MutableData("m", [[1,2,3]], dims=("one", "fixed"))

    # This is fine because the "fixed" dim is not resized
    pm.set_data({"m": [[1,2,3], [3,4,5]]})

    # Can't work because the "fixed" dimension is linked to a constant shape:
    # Note that the new data tries to change both dimensions
    pm.set_data({"m": [[1,2], [3,4]]})

@TimOliverMaier you could make that change here, ideally with a test case.

It is also relevant for #5751 and AFAIK @LukeLB is working on that at the moment.

[TimOliverMaier]

Thanks for the feedback I will look into this today. Also keeping an eye on #5751

[TimOliverMaier]

I rearranged the if-statements a little to cluster those relevant for the length_changed = True case. Furthermore the code now test if length_tensor has an owner. If not, it assumes the coords were set customly and logs a warning if the length was changed. I looked into the PR from @LukeLB addressing issue #5751 and added an if statement checking if dimension length is stored in TensorConstant.
Now I am looking for the best place for tests. Does anybody have a suggestion here?

[TimOliverMaier]

I added a test only to assert that it is possible to set new data with identical dimensions. I think additional tests should wait until a possible merge with #5763.

In models set_data stored coords differently from add_coord. I changed the line in set_data to store coords as tuple as well.

[michaelosthege]

I really like the informative messages!

Just a few rephrasing & typo changes... I'll apply them and merge it and then @LukeLB can close the coverage gaps after rebasing #5763.