search

mdw771 / adorym

Adorym: Automatic Differentiation-based Object Reconstruction with DynaMical Scattering
https://adorym.readthedocs.io/
25 stars 10 forks source link

Path settings for demo and production #10

Open chiahao3 opened 7 months ago

chiahao3 commented 7 months ago

Hi, thanks for sharing the code!

I was runing demo script "2d_ptychography_w_position_correction.py" and found some interesting path issues.

  1. ptychography.py line 224 h5py_path = f'{save_path}/demo/cone_256_foam_ptycho' seems to be hard-coded with the '/demo/cone_256_foam_ptycho` after {save_path} and this would break the i/o for other demo scripts
  2. ptychography.py line 397-404 if not os.path.exists('arrsize_{}_{}_{}_ntheta_{}'.format(*this_obj_size, n_theta)) seems to check and make the folder for rotation transformation coordinates at the working directory. For demo purpose I'd assume the working directory to be "adorym/demos", saving demo-specific folders at a common level is probably not ideal because all the demo scripts would be saving their folders here.
  3. With arrsize_256_256_1_ntheta_1 sitting under adorym/demos, it will also cause error in forward_model.py line 235, 451, 541, 657, 881 while they're all looking for the file under {save_path}/demo given coord_ls = read_origin_coords('{}/demo/arrsize_{}_{}_{}_ntheta_{}'.format(self.save_path, *this_obj_size, n_theta), theta_ls[this_i_theta], reverse=False). The save_path is defined as 'cameraman_pos_error' inside the demo script so we'll get NotFoundError.
  4. Lastly, the util.py that defines function save_rotation_lookup at line 492 seems to default the folder 'arrsize{}{}_{}ntheta{}' to be generated at working directiory as well, unless dest_folder is explicitly specified.

I wasn't really sure what'd be the best way to fix the path issues while maintaining the function in both demo and production scenario, but I'd guess we could first fix 1., and just save the 'arrsize' forlder under 'save_path', and then fix all 5 lines in forward_model.py by removing the /'demo', and prefix a {save_path} to the "dest_folder = " when we call save_rotation_lookup. Let me know if this makes sense or if I'm missing anything, thanks!

mdw771 commented 7 months ago

Thanks for pointing this out. The hardcoded paths were found in a pull request I merged a few months ago which I didn't realize. The quickest way to fix this is to roll back to the previous commit 31035d35cd016313bd8e80c79bcc9ba4aede9059, where it would follow user-specified working directories. I'll also reset the master branch back to that.