Skip to content

Conversation

Baharis
Copy link
Member

@Baharis Baharis commented Jul 7, 2025

Context

This week I decided to systematize creating the pets input file. In the current version of the code, the file is created in a brute-force way, by printing fixed lines into a file. The lines are completely hardcoded and, in contrast to e.g. very voluminous albeit rather primitive XDS file creation, customization options are very limited. Produced file can not be altered in any way by the user and it may be even illegal because there is no mechanism looking for duplicate keys.

The issue with making pets input is exacerbated when looking at the unused function write_pets2_inp. Apparently, at some point someone had a need to create slightly different pets input file, so in order to get to their goal while preserving the backwards-compatibility, they just copied existing functions and changed a few lines! This approach, seemingly popular for XDS (?) can in my opinion be excused at most on an experimental branch, not main!

In this PR, I suggest adding a new mechanism for creating PETS input files that utilizes objects in newly-added PETS_input_factory.py, in particular the PetsInputFactory itself. Instead of printing new lines, it is now sufficient to add them one by one to the factory, and then compile the result, which will produce the final file. In addition to being more readable at the higher level i.e. in the ImgConversion class, it can do things that the previous implementation could not:

  • If duplicate keyword is ever added, it is ignored (as it would break the input), and a warning is raised;
  • In addition to the generic keys, parts of the input file can be now included in camera config file:
    • config.camera.pets_prefix stores information that goes before = takes precedence over the generic information;
    • config.camera.pets_suffix stores information that goes after = can be overwritten by the generic information.

Both pets_prefix and pets_suffix can include Python3 format-style replacement fields i.e. {attrubute}. When compiling the PETS input, these will be replaced with class attributes of ImgConversion, if present, and – notably – ignored otherwise. With this combination, the user has significant agency over the shape of their PETS input file produced. For the need of my setup and experiments, I have added the following fields to my serval.yaml file:

pets_prefix: "noiseparameters 4.2 0\nreflectionsize 8\nI/sigma 5\nbin 1\ndetector asi\nbeamstop no\n"
pets_suffix: |
  cifentries
  _exptl_special_details
  ;
  {method} data collected using Instamatic.
  Tilt step:   	{osc_angle:.3f} deg
  Exposure:    	{headers[0][ImageExposureTime]:.6f} s per frame
  ;
  _diffrn_ambient_temperature          	?
  _diffrn_source                       	'Lanthanum hexaboride cathode'
  _diffrn_source_voltage               	200
  _diffrn_radiation_type               	electron
  _diffrn_radiation_wavelength         	0.0251
  _diffrn_measurement_device           	'Transmission electron microscope'
  _diffrn_measurement_device_type      	'FEI Tecnai G2 20'
  _diffrn_detector                     	'ASI Cheetah'
  _diffrn_measurement_method           	'{method}'
  _diffrn_measurement_specimen_support 	'Cu grid with amorphous carbon foil'
  _diffrn_standards_number             	0
  endcifentries
  
  badpixels
  359 32
  279 513
  endbadpixels

Note that the suffix has several fields that will be filled when the input file is compiled: {osc_angle:.3f}, {headers[0][ImageExposureTime]:.6f}, method. See the final file below to see how they have been handled.

Furthermore, the prefix contains two fields, noiseparameters 4.2 0 and reflectionsize 8, that the genetic factory sets as noiseparameters 3.5 38 and reflectionsize 20 by default. Because the fields from the prefix precede the default lines, the latter are ignored and my final file has the values correct for my detector, producing this nice PETS input file below. Mind that with this degree of freedom, it should be no longer necessary to have multiple separate functions writing pets output, but if you need them for some reason, making them is now also much simpler (see ImgConversion:ImgConversion.write_pets_inp).

# PETS input file for Electron Diffraction generated by `instamatic`
# Mon Jul  7 20:05:16 2025
# For definitions of input parameters, see: https://pets.fzu.cz/
noiseparameters 4.2 0
reflectionsize 8
I/sigma 5
bin 1
detector asi
beamstop no
                                                                     # default below, pets_prefix above
lambda 0.025079
aperpixel 0.001
phi 0.25
omega 231.6574538906956

imagelist
tiff/00000.tiff    -0.7500 0.00
tiff/00001.tiff    -0.2500 0.00
tiff/00002.tiff     0.2500 0.00
tiff/00003.tiff     0.7500 0.00
endimagelist
                                                                     # default above, pets_suffix below
cifentries
_exptl_special_details
;
Continuous-Rotation 3D ED data collected using Instamatic.
Tilt step:       0.500 deg
Exposure:        0.597142 s per frame
;
_diffrn_ambient_temperature              ?
_diffrn_source                           'Lanthanum hexaboride cathode'
_diffrn_source_voltage                   200
_diffrn_radiation_type                   electron
_diffrn_radiation_wavelength             0.0251
_diffrn_measurement_device               'Transmission electron microscope'
_diffrn_measurement_device_type          'FEI Tecnai G2 20'
_diffrn_detector                         'ASI Cheetah'
_diffrn_measurement_method               'Continuous-Rotation 3D ED'
_diffrn_measurement_specimen_support     'Cu grid with amorphous carbon foil'
_diffrn_standards_number                 0
endcifentries

badpixels
359 32
279 513
endbadpixels

Minor changes

  • config.camera.pets_prefix and pets_suffix can be used to dynamically expand the .pts made after an experiment.

Effect on the code

  • Improves the maintenance of the ImgConversion class by providing a general way to make pets input files.

Note

If would be better to have the pets keyword specification stored in a separate .csv rather than hard-coded in a string; this would be best done using importlib, which is unavailable in python 3.7 & 3.8 and I don't want to play with paths manually.

@Baharis Baharis requested a review from stefsmeets July 7, 2025 19:30
@Baharis Baharis self-assigned this Jul 7, 2025
@stefsmeets
Copy link
Member

stefsmeets commented Jul 8, 2025

Apparently, at some point someone had a need to create slightly different pets input file, so in order to get to their goal while preserving the backwards-compatibility, they just copied existing functions and changed a few lines!

This is the way 🚀 That would probably be me 😅 Thanks for completely over-engineering PETS2 generation, looking forward to reviewing this one!

Also, please add some basic tests for this new function, and documentation on how to customize the PETS data output.

Feel free to drop 3.7/3.8 as discussed in #130

@Baharis
Copy link
Member Author

Baharis commented Jul 8, 2025

Sure thing, I will add a few tests to assert the input is created properly as the config is changed. Nota bene I wrote this in such a way that without any affixes the input is identical with previous one. Given the freedom to drop 3.7/3.8, either no longer supported as of October 2024 (link), I will also move the csv to a separate file, but I'll try to leave most of the other code as is, other than maybe optimizing it with a few walruses 🦭 :).

@Baharis
Copy link
Member Author

Baharis commented Jul 8, 2025

In addition to the main content of this PR as already covered above, after the discussion I:

  • added five general tests that check the basic functionality of the new PetsInputFactory;
  • added a proper documentation for config.camera.pets_prefix and config.camera.pets_suffix;
  • added some pyproject.toml settings to assert the new csv resource if properly included
  • changed the range of supported python versions from 3.7–11 to 3.9–12 in docs, code, and CI;
    • while my instamatic runs on python 3.13 without issues, Github fails to resolve environment there yet; seeing how 3.13 is still in the bugfix stage, I removed Python 3.13 from the continuous integration tests for the time being.

IMO the new PETS_input_factory.py is not overly over-engineered; maybe it looks this way because I prefer to write heavily object-oriented code (as you can also in my new test file)? I might also have a distinct coding style, so for my own benefit I'd love to hear if any parts look too complex or should be refactored/simplified so that I can try to improve myself.

@Baharis
Copy link
Member Author

Baharis commented Jul 15, 2025

@stefsmeets Any specific comments or suggestions? Would you maybe prefer different testing, or maybe split Python version shift and PETS input into separate PRs? Sorry to urge you, but I have another PR lined up.

Copy link
Member

@stefsmeets stefsmeets left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Thanks for adding the tests, looks good to me. I added some small comments. Have a look and see what you think. Thanks for picking this up!

@Baharis
Copy link
Member Author

Baharis commented Jul 15, 2025

I addressed the suggestions; to summarize:

  • I kept the inline pets_prefix declaration because IMO it is a valuable example that you can do it this way;
  • As much as I'd love to, Isn't limiting python version to 3.12+ excessive? Pythons 3.9+ are still live, or do I misunderstand?
  • I made PETS_KEYWORDS uppercase as it should be;
  • I fixed the typo and removed [tool.setuptools.package-data] from the toml;
  • I moved the PetsInputWarning to the top; PETS_KEYWORDS and AnyPetsInputElement can't go higher because they rely on declarations before them.

@stefsmeets
Copy link
Member

Looks good to me, feel free to merge :-)

@Baharis Baharis merged commit aafc04e into instamatic-dev:main Jul 17, 2025
5 checks passed
@Baharis Baharis deleted the pets_input_writer branch July 17, 2025 18:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants