search

yoyololicon / kamui

MIT License
16 stars 4 forks source link

Bug in weights for ILP #6

Closed the-mysh closed 3 months ago

the-mysh commented 3 months ago

Hey, I've noticed what seems to be a typo in line 104 in core.py. The c which you use as argument to np.tile is not defined earlier. I'd appreciate if you could let me know what the correct argument here is so that I can use weights in my computations.

yoyololicon commented 3 months ago

Oh thank you for catching the bug! I'll push a fix real soon.

yoyololicon commented 3 months ago

fix in #7

the-mysh commented 3 months ago

Perfect, thanks for addressing this so fast!

the-mysh commented 3 months ago

Actually, sorry, something is not quite right still. I'm now getting this ValueError:

ValueError: Invalid input for linprog: c must be a 1-D array and must not have more than one non-singleton dimension

coming from scipy\optimize\_linprog_util.py:302.

Maybe it's a question of the dimensionality of the weights I'm passing? My weights are 2D, the same shape as the input phase dataset, should I reshape them somehow?

I'm using scipy 1.11.1 on Python 3.11.7 if that helps.

yoyololicon commented 3 months ago

Maybe it's a question of the dimensionality of the weights I'm passing? My weights are 2D, the same shape as the input phase dataset, should I reshape them somehow?

Yes, both differences and weights should be flattened and have the same shape. Are you using calculate_k directly or the wrapper unwrap_dimensional? I should add a conversion function to pass the weights from the latter to the former.

the-mysh commented 3 months ago

I'm using unwrap_dimensional, so yeah, I think I can see where the shape mismatch is coming from. I might switch to the unwrap_arbitrary later though, because I often have NaNs in my data and it seems unwrap_dimensional is having some trouble with that (it tends to think for a long time and then crash my Jupyter Notebook kernel if there are NaNs)

yoyololicon commented 3 months ago

Hi @the-mysh,

Regarding data with NaN, you can check issue #3.

the-mysh commented 2 months ago

Hi @yoyololicon, thanks for all the info. If you find some time to work on the weights shape conversion, I'll be excited to include it in my application!