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

Fix for multiplicity change in QEDbase #94

Merged

Conversation

AntonReinhard
Copy link
Member

@AntonReinhard AntonReinhard requested a review from szabo137 August 13, 2024 14:22
@AntonReinhard AntonReinhard force-pushed the fix/multiplicity-change branch from dff825b to 1287aec Compare August 13, 2024 17:36
@AntonReinhard
Copy link
Member Author

@SimeonEhrig I think there's a minor problem with the CI here: The docs fail because the CI_UNIT_PKG_URL_QEDbase is only used by the gitlab ci and not the github actions file. Because of that, the project can't be precompiled and the docs can't be built.

It's not too important because the update in QEDbase will be merged first and afterwards this problem will go away, but it would still be nice if it worked. Could that be easily added?

@SimeonEhrig
Copy link
Member

@SimeonEhrig I think there's a minor problem with the CI here: The docs fail because the CI_UNIT_PKG_URL_QEDbase is only used by the gitlab ci and not the github actions file. Because of that, the project can't be precompiled and the docs can't be built.

It's not too important because the update in QEDbase will be merged first and afterwards this problem will go away, but it would still be nice if it worked. Could that be easily added?

Do I understand it right that the problem is that incoming_spin_pols is defined in the dev branch of QEDbase and the documentation builds with the latest release?

@AntonReinhard
Copy link
Member Author

Do I understand it right that the problem is that incoming_spin_pols is defined in the dev branch of QEDbase and the documentation builds with the latest release?

incoming_spin_pols is defined in the feature branch that I linked with the CI_UNIT_PKG_URL_QEDbase: <commit> in the last commit message. But the documentation does build with the latest release which will be a problem because it should be the latest dev. The latest release will not get the function definition until after QEDbase is released again, and that should not be necessary.

@AntonReinhard AntonReinhard force-pushed the fix/multiplicity-change branch from 1287aec to 448c44b Compare August 15, 2024 13:53
@AntonReinhard
Copy link
Member Author

Ok yes, the feature in QEDbase has been merged but the documentation building still fails here because it uses the released version of QEDbase, not its dev branch.

We could change the docs/make.jl script to always depend on dev versions of QED packages, that might be the easiest way to circumvent this problem. It shouldn't get in the way with compatibility testing because that's what the gitlab CI is doing anyways. What do you think @SimeonEhrig ?

@SimeonEhrig
Copy link
Member

Please rebase. #95 should solve the CI build doc problem.

Use multiplicity in averaging_norm of Compton
@AntonReinhard AntonReinhard force-pushed the fix/multiplicity-change branch from 448c44b to 2dbe854 Compare August 21, 2024 09:17
@SimeonEhrig SimeonEhrig merged commit f82def5 into QEDjl-project:dev Aug 21, 2024
4 checks passed
@AntonReinhard AntonReinhard deleted the fix/multiplicity-change branch August 21, 2024 13:50
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