impy-project / chromo

Hadronic Interaction Model interface in PYthon
Other
30 stars 7 forks source link

Chromo release 0.4 #164

Closed jncots closed 11 months ago

jncots commented 1 year ago

This is a PR towards chromo's 1.0 release. I hope it should be quick. There are a couple of issues.

Sibyll- Redistribution Notice

This software is distributed as part of "Chromo" with the permission of the original author. Please refer to the original license for terms and conditions. No warranty provided.

should be fine.

What else should be done for release?

jncots commented 1 year ago

The above fixes #163 + tests.

jncots commented 1 year ago

Preliminary versions of licenses have been added as described above.

afedynitch commented 1 year ago

I added a couple of (hopefully) trivial commits. The summary is:

Hope that's it.

HDembinski commented 1 year ago

@afedynitch Regarding the version number 1.0, please see #165 why I argue against going for a 1.0 at this point.

1.0 means you have a stable product with a stable API, while 0.x indicates that the API is not finalized yet. Our API is not fixed yet.

afedynitch commented 1 year ago

@afedynitch Regarding the version number 1.0, please see #165 why I argue against going for a 1.0 at this point.

1.0 means you have a stable product with a stable API, while 0.x indicates that the API is not finalized yet. Our API is not fixed yet.

Oh, I totally agree with that. It will be changed. But since there is an interface change in kinematics between the pypi and even the current main, we should bump it up to 0.4. There are also now some additional changes that would justify upping the minor version.

I'm looking into #160 now, and there are some warnings in test about decays in PHOJET & DPMJET, which shouldn't be there. All of these models can decay the same stuff as PYTHIA6, since this is what is used for decays.

I would prefer getting the final state stuff unified, it is quite important. Then merge it into main, and rebase of FF this PR onto the latest main. Target time is for this one is some time next week, since we removed the --pre flag from the poster ;).

afedynitch commented 1 year ago

So in principle, this seems complete. We should try to finalize the #169 PR, and possibly #167, and merge those before proceeding here.

jncots commented 1 year ago

Dear @HDembinski, @afedynitch, we currently have 6 open PRs that are nearly ready for merging.

Proposed Merging Order:

jncots commented 1 year ago

@HDembinski, while working on #170 I noticed that it would be good to have the following changes in graphviz and HepMC:

Should I submit both of these issues to Hepmc github?

HDembinski commented 1 year ago

graphviz don't show loops, i.e. multilple particles cannot originate from one vertex and then merge to another. If such situation occurs, then only there first particle is displayed, all other are missing

If I remember correctly, this is not only an issue with graphviz but already with HepMC3, which models the event as a directed acyclic graph, so loops are not supported. I think it is unfixable and we need a workaround, as before. So far I used the workaround to remove all the preshower history when converting from a Pythia event to HepMC3. Is there an issue with this workaround after your changes?

HDembinski commented 1 year ago

Should I submit both of these issues to Hepmc github?

I will fix the indexing issue in pyhepmc now, no need to add an issue. I cannot fix the other thing with the cyclic graphs, so it won't help to add an issue there.

jncots commented 1 year ago

Is there an issue with this workaround after your changes?

The problem with cyclic graph appears in Phojet and Dpmjet. As a workaround, when conversion to hepmc occurs, only the beam and final particles are used. I hope that we will figure out how to write the whole history later.

HDembinski commented 1 year ago

The problem with cyclic graph appears in Phojet and Dpmjet. As a workaround, when conversion to hepmc occurs, only the beam and final particles are used. I hope that we will figure out how to write the whole history later.

This is worse than the workaround I use for Pythia. In Pythia, I only have to remove the preshower information, which is acceptable, because as an experimentalist you cannot exploit this information. The decay history of short or long-lived particles is kept, however, which one frequently needs as an experimentalist. It would be great to keep the decay history in DPMJet / PhoJet as well.

jncots commented 1 year ago

This is worse than the workaround I use for Pythia.

I agree. I will try to fix it.

afedynitch commented 1 year ago

Dear @HDembinski, @afedynitch, we currently have 6 open PRs that are nearly ready for merging.

Proposed Merging Order:

  • [ ] *Sibyll - Approved** ✅ ....

The order is fine. Please start merging one by one

HDembinski commented 1 year ago

@jncots A release of pyhepmc with the required extension is currently running https://github.com/scikit-hep/pyhepmc/releases/tag/v2.13.0. If all goes well, it will be online on PyPI in about an hour. To use it in chromo, you need to use the new keyword fortran=False wherever you call the GenEvent.from_hepevt method.

jncots commented 1 year ago

@jncots A release of pyhepmc with the required extension is currently running https://github.com/scikit-hep/pyhepmc/releases/tag/v2.13.0.

@HDembinski, excellent! Thank you.

HDembinski commented 1 year ago

@jncots There were some issues with the release job due to outdated CI action scripts, but now the updated pyhepmc package is available on PyPI. https://pypi.org/project/pyhepmc/

jncots commented 11 months ago

@HDembinski do we need python3.8 on Linux? If yes, then https://pypi.org/project/pyhepmc/2.13.2/#files doesn't have version for python3.8 and it should be added.

jncots commented 11 months ago

@afedynitch, do we need to fix #177 and #178 for the release?

afedynitch commented 11 months ago

@afedynitch, do we need to fix #177 and #178 for the release?

Does or prevent HEPMC+RIVET from functioning?

jncots commented 11 months ago

Does or prevent HEPMC+RIVET from functioning?

No, HEPMC+RIVET works fine. For Phojet there is no history, but it is not so critical. For Dpmjet intermediate particles has wrong energies, so they are end up in HEPMC.

afedynitch commented 11 months ago

Does or prevent HEPMC+RIVET from functioning?

No, HEPMC+RIVET works fine. For Phojet there is no history, but it is not so critical. For Dpmjet intermediate particles has wrong energies, so they are end up in HEPMC.

Ok, then this is not critical for this release. We’ll do it in the next release.

I’ll go through the review a bit later today, so we can merge this soon

afedynitch commented 11 months ago

Overriding @HDembinski review, because the main criticism was directed toward version 1.0. Now it’s 0.4, so should be fine