davlars / ad-skull-reconstruction

Repository for reconstruction of simulated skull CT data for AD project
2 stars 5 forks source link

New stride for RectGrid in odl causes bug? #35

Open aringh opened 7 years ago

aringh commented 7 years ago

I think that this commit in odl causes crash in the pickled geometries corresponding to this package. The pickled geometries do not have a self.__stride = None in the init so for example power_method_opnorm does not work.

(I am not sure what stride does, but the crash occurred in power_method_opnorm for me)

aringh commented 7 years ago

The quick-fix for this, if anyone like me has already pulled the newest commits from odl, is to simply check out the parent commit 3fd52ace... and run the code from there.

I.e., in your odl repo, simply do

[...]/odl$ git checkout 3fd52ace

You will probably get a message like

Note: checking out '3fd52ace'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by performing another checkout.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -b with the checkout command again. Example:

  git checkout -b <new-branch-name>

HEAD is now at 3fd52ac... MAINT: Minor performance improvement to dft_postprocess_data

which you can basically ignore. When you want to go back to any other branch, simply go

[...]/odl$ git checkout name_of_branch

for example git checkout master.

davlars commented 7 years ago

Hmmmm. Assuming that self.__stride__ = None is a new feature of e.g. HelicalFanFlat in odl I guess this will be a recurring problem until we update the geometries? The question is, should we update the geometries with the latest odl-commit? Or is this still WIP?

adler-j commented 7 years ago

This is remarkably irritating, and for now just use the workaround.

In the long term (as in within a week honestly) we have to get the long term fix shipped, and I've made an ODL issue w.r.t. this here https://github.com/odlgroup/odl/issues/1028.

Another option is to follow the advice here and perhaps move to JSON, it is less flexible but much easier to modify in these cases. It still doesn't change the core issue but simply makes it easier to handle.

davlars commented 7 years ago

Right. Great that you've made an odl issue on the topic. Maybe we can await any soon-to-come changes on the odl side, and if that doesn't happen JSON seems like a fair alternative.