Introduction of F4Enix dependency for MCNP input and output parsing

[AlbertoBittes]

Description

Code was refactored to introduce f4enix as a dependency and to avoid duplication.

Fixes #293

Additional dependencies introduced.
f4enix >= ...

Type of change

Please select what type of change this is.

• Bug fix (non-breaking change which fixes an issue)
• New benchmark
  • Non-breaking change which entirely uses exisiting classes, structure etc
  • Breaking change which has implemented new/modified classes etc
• New feature
  • Non-breaking change which adds functionality
  • Breaking change fix or feature that would cause existing functionality to not work as expected

Other changes

• This change requires a documentation update

Some parts in the "General OOP Scheme" of documentation had to be modified.

Testing

Tests have been massively refactored. All the remaining tests are passing. Removed functions are tested in f4enix.

Checklist:

• My code follows the style guidelines of this project
• I have performed a self-review of my code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• General testing
  • New and existing unit tests pass locally with my changes
  • Coverage is >80%

[dodu94]

Thanks Alberto, good job!

[dodu94]

Can you please link these to the F4Enix API reference? you can find them navigating here: https://f4enix.readthedocs.io/en/latest/_autosummary/f4enix.input.html#module-f4enix.input

[dodu94]

Can you double check if these are used anymore? I believe we removed the automatic API generation

[dodu94]

see above comment

[dodu94]

Can you explain what is happening here?

self.out.stat_checks = self.out.get_statistical_checks_tfc_bins()
self.out.stat_checks = self.out.assign_tally_description(
    self.out.stat_checks, self.mctal.tallies)

why it is overridden?

[dodu94]

Have you double checked that dic["x"] is actually what exits from mcnp and was not modified at some point to be the midpoints?

[dodu94]

1. Why is this needed?

def __init__(self):
    raise RuntimeError("SphereTallyOutput cannot be instantiated")

2. def _get_tallydata(self, mctal): in the SphereOutput produces a different DataFrame with respect to the one of F4Enix? Do we really need to keep it or we can use that one? Also because now we are reading the tallies twice since this is done also in the super().__init__()

[alexvalentine94]

I guess SphereTallyOutput is never instantiated on its own, now we are differentiating across codes so makes sense to define it as an abstract class. I think there are neater way to do this however by defining each method as an @AbstractMethod. However thiscan be for future polishing.

[dodu94]

It seems that the logic of the D1S translation has been changed here. What JADE did in the past (here) was to take all possible reactions, check which daughters are in the irradiation file and then convert all the parents found in the mcnp input with the activation library, the others with the transport. That is why it was outputting the list of new irradiations and reactions. smart_translate() (here) checks the reaction file and use the list of reactions there as input to get the parents where activation libraries should be applied in the mcnp input.

In general, in D1S input the irradiation file has to be provided, since time factor cannot be computed by JADE or F4Enix. This means that if you want to keep the same behaviour as before (and we want), first we need to call get_reaction_file() in order to get all reactions leading to those daughters, and only after that we can call smart_translate().

Once this is done, I think we can delete also this part of the code:

 newirradiations = []
            available_daughters = self.d1s_inp.irrad_file.get_daughters()
            for reaction in self.d1s_inp.reac_file.reactions:
                if reaction.daughter in available_daughters:
                    # add the correspondent irradiation
                    irr = self.d1s_inp.irrad_file.get_irrad(reaction.daughter)
                    if irr not in newirradiations:
                        newirradiations.append(irr)
            self.irrad.irr_schedules = newirradiations
            self.react.reactions = newreactions

            self.react.reactions = self.d1s_inp.reac_file.reactions

please add a suitable test for this. Like for instance checking the ITER cylinder benchmark. The reactions in the react file should depend on the library as the application of the activation lib in the input.

[dodu94]

Thanks Alberto, all good from my side now. @alexvalentine94 can start his review now.

[alexvalentine94]

Thanks for this great effort. This has made the code base much more concise and a lot neater in parts. I have made some comments, nothing particularly major.

[alexvalentine94]

Thanks Alberto for adressing the comments. I am happy with the revisions and that the code is suitable for merging.