Minimal initial commit of FreePACT code

[ParsonsRD]

This is a commit of a simple implementation of the FreePACT code. Updates to the ImPACT code are included from #2705

[maxnoe]

I'd add a new extra here called impact or freepact that depnds on tensorflow and iminuit (you need both, right?). So that people can do pip install ctapipe[impact] and that gives them a working environment for impact.

In all you can then depend on ctapipe[impact].

[maxnoe]

It seems current tensorflow does not yet support python 3.14. You can remove the impact requirements from all into their own extra as written above, and only add the extra to certain builds e.g. the mamba 3.13 or pip 3.12 build.

[maxnoe]

I.e. after adding the impact extra, you add it in the list here in the last line to include it in the 3.12 tests:

ctapipe/.github/workflows/ci.yml

Lines 56 to 61 in 3e1717f

	- name: Linux (3.12, pip, coverage)
	os: ubuntu-24.04
	python-version: "3.12"
	install-method: pip
	extra-args: ["codecov"]
	extras: tests,all

[ParsonsRD]

@maxnoe I'm a bit lost what to do here, I can add the tensorflow dependencies for the older versions of python but the failing CI build of 3.14 seems to cancel all the others.

[maxnoe]

@ParsonsRD you need to make the import of tensorflow actually optional in the code.

I.e. you need to replace code like this:

import tensorflow as tf

class FreePactReconstructor:
    def __init__():
         # do something with tf

with something like:

try:
    import tensorflow as tf:
except ModuleNotFoundError:
    tf = None


class FreePactReconstructor:
    def __init__(self):
        if tf is None:
            raise OptionalDependencyMissing("tensorflow")

See e.g. how this is done here for matplotlib:

ctapipe/src/ctapipe/visualization/mpl_camera.py

Lines 102 to 106 in 3e1717f

	try:
	import matplotlib.pyplot as plt
	from matplotlib.collections import PatchCollection
	except ModuleNotFoundError:
	raise OptionalDependencyMissing("matplotlib") from None

The important thing is that the modules need to be importable without the optional depedency being installed and the error is moved to runtime of the algorithm.

[ParsonsRD]

Ugh sorry yes that should have been obvious. I blame on being very tired today :-).

[maxnoe]

tf and keras does not seem to play nice with the latest version of numpy. But I can't get my head around this error...

This is not actually an error but only a deprecation warning. Our test setup by default turns those into exceptions.

You can keep this particular warning as a warning by adding a line like this to pyproject.toml in the pytest.filterwarnings option:

ctapipe/pyproject.toml

Lines 158 to 171 in 3574fb1

	filterwarnings = [
	"error::DeprecationWarning",
	"error::astropy.utils.exceptions.AstropyDeprecationWarning",
	"error::ctapipe.utils.deprecation.CTAPipeDeprecationWarning",
	"error::ctapipe.instrument.FromNameWarning",
	"error::ctapipe.core.traits.NoneDefaultNotAllowedWarning",
	"error::tables.UnclosedFileWarning",
	"ignore:`np.MachAr` is deprecated:DeprecationWarning",
	"ignore::ctapipe.core.provenance.MissingReferenceMetadata",
	# inside matplotlib, already fixed but occurs in "oldest-deps" tests
	"ignore:'parseString':DeprecationWarning:matplotlib",
	"ignore:'resetCache':DeprecationWarning:matplotlib",
	"ignore:'oneOf':DeprecationWarning:matplotlib",
	"ignore:'leaveWhiteSpace':DeprecationWarning:matplotlib",

"ignore:__array__ implementation doesn't accept a copy keyword:DeprecationWarning"

[maxnoe]

For the warnings that come from importing protobuf via tensorflow, I have these in another project:

    # deprecation warning from protobuf with python 3.12
    "ignore:Type .* uses PyType_Spec with a metaclass that has custom tp_new:DeprecationWarning",
    "ignore:datetime.datetime.utcfromtimestamp\\(\\):DeprecationWarning",

[maxnoe]

you need one more colon here. It is <action>:<regex for message>:<category>:

https://docs.python.org/3/library/warnings.html#the-warnings-filter

[ParsonsRD]

Thanks, I couldn't find an explanation of this anywhere and couldn't find a way of testing it without pushing commits

[maxnoe]

These warnings are triggered in the oldest-deps build, where we test against the oldest still supported versions of our dependencies. You can test locally by installing the modules at the versions used in CI for this:

ctapipe/.github/workflows/ci.yml

Lines 155 to 157 in 3574fb1

	- if: contains(matrix.extra-args, 'oldest-deps') && contains(github.event.pull_request.labels.*.name, 'documentation-only') == false
	run: |
	micromamba install -n cta-dev -y numpy=1.26 numba=0.59.0 scipy=1.15 astropy=6.1 matplotlib=3.8 eventio=1.11.6

[maxnoe]

In this case, it's matplotlib=3.8 raising these warnings.

[maxnoe]

If you don't mind, I'll push a commit to update the pyproject

[maxnoe]

We use the bibtex extension for sphinx, please add the reference to the docs/references.bib file and use:

:cite:p:`<bibtex key>`

[maxnoe]

Please remove the impact extra from the CI job for minimal-dependencies, the task of this job is to test ctapipe without any of the optional dependencies installed.

[maxnoe]

Please remove the impact extra from the CI job for minimal-dependencies, the task of this job is to test ctapipe without any of the optional dependencies installed.

done

[maxnoe]

The tests are passing now. You do not need to worry about the pending check, that comes from branch protection rules requiring a certain job which is no longer here since the "impact" extra is now included in that step.

Once we are happy to merge here, I'll update the branch protection rules to this new definition.

[ParsonsRD]

OK great, thanks for the help in working through the CI issues. I will work through the sonarqube warnings and fix where appropriate. I would also like to have @gschwefer check through this and make sure it is fully compatible with the training files he has created.

The other major missing work is to distribute a training code, which is currently a set of notebooks. This can come in a separate PR I think. Or we save it for a larger commit where we spin impact and freepact off into a plugin, which I think should probably be the ultimate goal (assuming Georg feels the same way).

[maxnoe]

You create a full tableloader here only to get the subarray? If all you need is the subarray, just to SubarrayDescription.from_hdf(...)

[ctao-sonarqube]

Quality Gate failed

Failed conditions
10 New issues
54.3% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

[maxnoe]

In general, you do not need to transform exceptions into sys.exit calls, indeed it is better to not do so. This is handled by the parent Tool class in its execution logic.

[ctao-sonarqube]

Quality Gate failed

Failed conditions
3 New issues
53.1% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

[ParsonsRD]

After speaking with @kosack and @maxnoe I created an ImPACT and FreePACT plugin here:
https://github.com/ParsonsRD/FreePACT-Reconstructor-plugin

If this seems like a better approach, we could close this request.

[maxnoe]

@ParsonsRD looks mostly good. Didn't have a chance yet to try it out though.

Some observations:

• Claiming compatibility with ctapipe >= 0.17 seems a bit... optimistic, did you test with versions that old?
• In the README, you tell people to instantiate via "Reconstructor.from_name, however, I think this is not really meant for user code but for use inside of ctapipe to discover the plugin classes. Users should probably just just ctapipe-processconfiguring theFreePACTReconstructor, or use the ShowerProcessor` or instatiate the class directly from the plugin.