search

hqucms / weaver-core

Streamlined neural network training.
MIT License
44 stars 54 forks source link

Update tools.py #1

Closed StephenChao closed 2 years ago

StephenChao commented 2 years ago

Days ago when I tried to run a train, this line(https://github.com/hqucms/weaver-core/blob/646a9408b776e7dcf9c21324ff2b28153b472162/weaver/utils/data/tools.py#L116 ) raised an error saying 'a coordinate must be of the type int or float' .

I found a discussion about this very problem here: https://gitter.im/Scikit-HEP/vector . Like documented in this discussion, it turns out to be the difference between 'vector' module, in version 0.8.5 it's fine but version 0.9.0 and later adds a type-check.

Anyway using 'vector.zip' still works. I don't know whether https://github.com/hqucms/weaver-core/blob/646a9408b776e7dcf9c21324ff2b28153b472162/weaver/utils/data/tools.py#L110 and https://github.com/hqucms/weaver-core/blob/646a9408b776e7dcf9c21324ff2b28153b472162/weaver/utils/data/tools.py#L101 will raise the same error since they also used 'vector.Array' but I didn't met the same problem. I can make another pull request when I confirm it.

hqucms commented 2 years ago

Hi @StephenChao -- Thanks a lot for the pull request!

I would suggest to make the same change to all cases using vector.Array.

In addition, I think we also need to update the vector version requirement here: https://github.com/hqucms/weaver-core/blob/646a9408b776e7dcf9c21324ff2b28153b472162/requirements.txt#L11 from >=0.8.5 to >=0.9.0, right?

hqucms commented 2 years ago

Integrated with https://github.com/hqucms/weaver-core/commit/6811e2db51dbcb22367102ac14647de3af1dc5ec.

StephenChao commented 2 years ago

Yes I think so, thanks very much!