Problems with pars in notebooks

[jadball]

Hi James,

Just to give you some feedbacks about your very new updates on the notebook.

Seen with user measurement data, found two small issues:

1) in the indexing notebook, we have to do:

cf_4d.parameters.loadparameters(par_file)

cf_4d.updateGeometry()

before ucell = ImageD11.unitcell.unitcell_from_parameters(cf_4d.parameters)

It seems cf_4d = ds.get_cf_4d() does not get parameters. Probably this is because ds does not contain parameters any more.

2) in the sinogram_map notebook:

write_h5(ds.grainsfile, grainsinos, overwrite_grains=False) currently has a different key from the old version which uses "write_grains_too" ?

So I guess the notebook needs to be updated to the new key name.

Cheers,

Haixing

Originally posted by @haixing0a in 3eb8107

[jadball]

@haixing0a can you please confirm that you're using the latest notebooks from git master? I'm pretty confident that I tested these things before merging. When you run the following lines, the cf_4d should have the correct parameters assigned:

cf_4d = ds.get_cf_4d()
ds.update_colfile_pars(cf_4d, phase_name=phase_str)

[jonwright]

I broke that. Not sure if I remember why?

[jonwright]

Or it might not be me - I can't read git history. Looks like this one:

https://github.com/FABLE-3DXRD/ImageD11/commit/c066ed77047ec2db61f4062edde7b1da7d075257#diff-4b57e38df8ad163dfd1918781b95543a751ccf94e23e9849da31865c626ea033L631

Anyway - please don't revert - when I open the cf_4f from ImageD11.sinograms.properties I want the raw peaksearch output with nothing added. Call dataset.update_colfile_pars or just fix the notebook to load and use the parameters.

[jadball]

The current implementation in dataset and the current notebooks should work fine, hence my confusion here. I need more info about how exactly this is breaking to know whether a fix is needed

[jadball]

I suspect an older notebook is being used

[jonwright]

Unrelated, but before I forget: the phase_str should not be used for loading the cf_4d.

https://github.com/FABLE-3DXRD/ImageD11/blob/master/ImageD11/nbGui/S3DXRD/1_S3DXRD_index.ipynb

# [cell ~ 7]
cf_4d = ds.get_cf_4d()
ds.update_colfile_pars(cf_4d, phase_name=phase_str)

[jadball]

I'm not sure I know what the problem is here. When we call ds.get_cf_4d(), we don't touch the parameters at all.

When we call ds.update_colfile_pars(cf_4d, phase_name=phase_str), we are updating the parameters object for cf_4d using pars from disk (via ds.parfile). We specify a phase_str so that we load the correct phase parameters into the parameters object - if phase_str is not given, it will take the first phase from the json file instead.

[haixing0a]

James, sorry, I think I was using a notebook from the version 1.5 week ago which still uses .par for parameters. I realized the the newest notebook needs json file to contain different pars now.

[jadball]

@haixing0a no problem, easy to do! I think between the three of us we need to think of better ways of setting up the notebooks for a new experiment - perhaps moving them to a new git repo, and always having a checkout of the latest notebooks somewhere in inhouse that we can copy into SCRIPTS?

[jonwright]

In general: the peak positions in cf_4d only need to know the detector parameters - then saving the cf_4d will not depend on the phase id.

It will mean adding a unitcell or phase_name argument for select_ring_peaks_by_intensity or indexer_from_colfile, etc. I am thinking the old style parameters inside of the columnfile are not the right place to store the unit cell in the future. We need this for backward compatibility, but it doesn't feel right going forward.

[jadball]

It will mean adding a unitcell or phase_name agument for select_ring_peaks_by_intensity or indexer_from_colfile, etc.

This is what I did for now - see 06fae6b2126ac6700a2c71be6aa5d5af985ea087

I agree that columnfiles being phase-aware is confusing and messy. Personally I think columnfiles should not have parameters at all, to avoid duplication of parameters objects in memory (where you modify one but forget about the other, etc etc)

[jonwright]

The reason to put the detector parameters in with the peaks is so that you can have several columnfiles and each one has it's own calibration (e.g. hydra arrangements). Loading one parameters object and then saving a pointer avoids the duplication (e.g. colfile.parameters = shared_parameters ).