Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Release 0.3.0 to main #121

Merged
merged 18 commits into from
Oct 24, 2024
Merged

Conversation

AntonReinhard
Copy link
Member

Release the new features to main in a new release process.

This is a release to main, do not squash or rebase

AntonReinhard and others added 10 commits July 12, 2024 15:24
Merge release back into dev, main merge was in QEDjl-project#99
…project#106)

Adds the functions `incoming_spin_pol`, `outgoing_spin_pol`, and
`spin_pol` to the `AbstractProcessDefinition` interface. These can be
implemented by specific processes, or rely on the provided default
implementation which returns `AllSpin` or `AllPol` for each particle.

Closes QEDjl-project#105
Adds a second directory for all implementations on the abstract
interfaces. Type implementations go into QEDcore. Since I plan to add
some more functionality on the AbstractProcessDefinition, this seemed
necessary to keep structure.

Also deletes a left-over duplicate file
Rewrite momenta function without broadcast, because GPUs do not like
broadcasts in their kernels.
This is essentially ported from
QEDjl-project/QEDprocesses.jl#69 since this
implementation moved here since that PR was opened.
Fix CI branch names so `main` deploys docs as well.
…EDjl-project#118)

As the title says, this adds an iterator to yield all possible
combinations of spins and polarizations allowed by a process' set
`spin_pols()`. For example:

```Julia
julia> using QEDbase; using QEDcore; using QEDprocesses;

julia> proc = ScatteringProcess((Photon(), Photon(), Photon(), Electron()), (Photon(), Electron()), (SyncedPolarization(1), SyncedPolarization(2), SyncedPolarization(1), SpinUp()), (SyncedPolarization(2), AllSpin()))
generic QED process
    incoming: photon (synced polarization 1), photon (synced polarization 2), photon (synced polarization 1), electron (spin up)
    outgoing: photon (synced polarization 2), electron (all spins)


julia> for sp_combo in spin_pols_iter(proc) println(sp_combo) end
((x-polarized, x-polarized, x-polarized, spin up), (x-polarized, spin up))
((y-polarized, x-polarized, y-polarized, spin up), (x-polarized, spin up))
((x-polarized, y-polarized, x-polarized, spin up), (y-polarized, spin up))
((y-polarized, y-polarized, y-polarized, spin up), (y-polarized, spin up))
((x-polarized, x-polarized, x-polarized, spin up), (x-polarized, spin down))
((y-polarized, x-polarized, y-polarized, spin up), (x-polarized, spin down))
((x-polarized, y-polarized, x-polarized, spin up), (y-polarized, spin down))
((y-polarized, y-polarized, y-polarized, spin up), (y-polarized, spin down))

julia> length(spin_pols_iter(proc))
8
```
The above is also a `jldoctest`.

As a side-note I also added an alias of `SyncedPol` to
`SyncedPolarization`.

The code is not incredibly concise and also not incredibly fast, but for
the reasonable cases that I tested `@benchmark` reports well under 1ms.
Since I don't think this iterator would be the critical path of anything
this should be fine.
The only problem I could see is that due to everything using `Tuple`s in
its arguments, the compile time is relatively large. If this becomes a
problem we could change it to using `Vector`s instead, likely trading
some runtime for much better compile time.

Fixes QEDjl-project#107
@AntonReinhard
Copy link
Member Author

The doc building job fails because it's not setting the dev dependencies of the QEDproject packages, see the if condition:

      - name: set dependencies to dev branch version
        if: (github.event_name == 'push' && github.ref_name != 'main') || (github.event_name == 'pull_request' && github.base_ref != 'main')

I don't think this is intentional as we should always build the docs against the dev versions or they will fail in cases like this one, where breaking changes are introduced that cannot be released in the downstream packages yet.
@SimeonEhrig should I just delete the github.ref_name != 'main' conditions?

Copy link
Member

@szabo137 szabo137 left a comment

Choose a reason for hiding this comment

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

There are several broken integration tests. It seems to be a compatibility problem. I suggest to update the packages depending on QEDbase.jl first, by replacing the pinned [compat] entry with a lower bound, e.g. "^0.2". After that, the integration tests in this PR should pass, and we can proceed with the release process.

@AntonReinhard
Copy link
Member Author

There are several broken integration tests. It seems to be a compatibility problem. I suggest to update the packages depending on QEDbase.jl first, by replacing the pinned [compat] entry with a lower bound, e.g. "^0.2". After that, the integration tests in this PR should pass, and we can proceed with the release process.

^ is already the default specifier according to https://pkgdocs.julialang.org/v1/compatibility/. In any case, the point of the compat is to specify which versions are compatible, and allowing "any version higher than x" goes against that, when there can be breaking changes in the future that would automatically be included.

The problem isn't really with the compat entries, they are as they should be, the problem is that we want to "override" the compat entries for the testing. I think the simplest solution might be to modify the Project.toml in the package that the integration test is being created for, by deleting the currently tested package's compat entry entirely, so the resolve can't refuse installing it.

@szabo137
Copy link
Member

^ is already the default specifier according to https://pkgdocs.julialang.org/v1/compatibility/. In any case, the point of the compat is to specify which versions are compatible, and allowing "any version higher than x" goes against that, when there can be breaking changes in the future that would automatically be included.

I see, you are right.

The problem isn't really with the compat entries, they are as they should be, the problem is that we want to "override" the compat entries for the testing. I think the simplest solution might be to modify the Project.toml in the package that the integration test is being created for, by deleting the currently tested package's compat entry entirely, so the resolve can't refuse installing it.

I understand the problem, and I think your suggested solution will work. However, I don't know if it is a good idea to directly change the Project.toml, even just for testing. Maybe this would be a good question for discourse 🤔

@SimeonEhrig
Copy link
Member

@SimeonEhrig should I just delete the github.ref_name != 'main' conditions?

"The biggest problem of the software development - User!". You constantly breaks my assumptions about the CI. I didn't thought about the possibility, that the documentation could have different dependencies compare to the actual software. Therefore the check forces the same requirement, that the main branch version needs to work with released packages.

But I see the problem. If we force this also for the documentation the QED dependency graph becomes an unmanageable thing, where everything is more or less depend on everything. Therefore I think we can relax the requirements for the documentation. But this also means, our documentation documents feature in the release which are maybe not working yet because a dependent QED project was not released in the required version yet.

@szabo137 What do you think?

@AntonReinhard
Copy link
Member Author

There are two separate problems here, one is the documentation build, the other are the integration tests.

For the docs building:

"The biggest problem of the software development - User!". You constantly breaks my assumptions about the CI. I didn't thought about the possibility, that the documentation could have different dependencies compare to the actual software. Therefore the check forces the same requirement, that the main branch version needs to work with released packages.

I think the problem is that the docs can have dependencies to packages that are otherwise downstream of the package itself, and therefore needs to be considered similar to the integration tests instead of the unit tests. This does mean, as you say, that there might be docs for a time that aren't reproducible with only released packages, at least until all required downstream packages are released again.
In any case, we now always build the integration tests with dev dependencies and have the release version integration tests only as information for the developer, so always building the docs with dev dependencies too is just streamlining it in my opinion. So simply deleting the github.ref_name != 'main' check I mentioned should be an easy fix here that also makes sense.

Now for the integration tests

I think this is the bigger problem because as I said, we somehow need to force the downstream packages to accept the specific version of the package that we give it, regardless of what the compat tells it to use. The ideal scenario would be if the Julia's package manager allowed some kind of flag to override the compat entry, but from a quick google I can't find anything like this. Maybe it would be possible to steal borrow some code from Julia's CompatHelper to find and change the compat entry manually when generating the integration tests.

@SimeonEhrig
Copy link
Member

There are two separate problems here, one is the documentation build, the other are the integration tests.

For the docs building:

"The biggest problem of the software development - User!". You constantly breaks my assumptions about the CI. I didn't thought about the possibility, that the documentation could have different dependencies compare to the actual software. Therefore the check forces the same requirement, that the main branch version needs to work with released packages.

I think the problem is that the docs can have dependencies to packages that are otherwise downstream of the package itself, and therefore needs to be considered similar to the integration tests instead of the unit tests. This does mean, as you say, that there might be docs for a time that aren't reproducible with only released packages, at least until all required downstream packages are released again. In any case, we now always build the integration tests with dev dependencies and have the release version integration tests only as information for the developer, so always building the docs with dev dependencies too is just streamlining it in my opinion. So simply deleting the github.ref_name != 'main' check I mentioned should be an easy fix here that also makes sense.

Now for the integration tests

I think this is the bigger problem because as I said, we somehow need to force the downstream packages to accept the specific version of the package that we give it, regardless of what the compat tells it to use. The ideal scenario would be if the Julia's package manager allowed some kind of flag to override the compat entry, but from a quick google I can't find anything like this. Maybe it would be possible to steal borrow some code from Julia's CompatHelper to find and change the compat entry manually when generating the integration tests.

Well, the integration tests clones the source code of the project to test. The Project.toml is just a toml file. Therefore there is nor reason why we cannot modify the file before we instantiate the project.

@AntonReinhard
Copy link
Member Author

It looks like we have the same compat problem in the documentation too...

@SimeonEhrig
Copy link
Member

It looks like we have the same compat problem in the documentation too...

I think this is something what we should discuss tomorrow. I was also thinking if the correct solution is to modify the compat entries of the packages on the dev branch version.

I already set up the deploy key and the private ssh key as a repository
secret.
szabo137 and others added 3 commits October 9, 2024 17:36
Following the suggestion made here:
QEDjl-project/QEDcore.jl#45 (comment)

---------

Co-authored-by: Uwe Hernandez Acosta <[email protected]>
Co-authored-by: Anton Reinhard <[email protected]>
- [x] Fix jldoctests
- [x] finish API reference
- [x] Write tutorial for PSP
- [x] Write tutorial for stateful particles
- [x] Write tutorial for Lorentz vectors
- [x] Write tutorial for particle interface
- [x] Write tutorial for processes

---------

Co-authored-by: Uwe Hernandez Acosta <[email protected]>
SimeonEhrig and others added 2 commits October 23, 2024 23:05
…project#125)

Use overworked CI script to setup dev branch Julia environment. The CI
scripts allows to resolve circular dependencies and modify compat
entries of dependencies, if own project version was updated.
@AntonReinhard AntonReinhard marked this pull request as draft October 23, 2024 22:10
@AntonReinhard
Copy link
Member Author

I will have to update the Changelog again, other than that this should (finally) be ready to merge

@SimeonEhrig
Copy link
Member

@AntonReinhard The CI passed 🥳

@AntonReinhard AntonReinhard marked this pull request as ready for review October 24, 2024 07:17
@szabo137
Copy link
Member

@SimeonEhrig @AntonReinhard many thanks for you effort making this running (again) 🥳

Copy link
Member

@szabo137 szabo137 left a comment

Choose a reason for hiding this comment

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

LGFM ✌️

@AntonReinhard AntonReinhard merged commit 4c0ec58 into QEDjl-project:main Oct 24, 2024
4 checks passed
@AntonReinhard AntonReinhard deleted the release-0.3.0 branch October 24, 2024 09:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants