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

Synchrotron losses time evolution #157

Merged

Conversation

grzegorzbor
Copy link
Contributor

The ParticleDistribution.evaluate_time() function added.
It's a basic method that accepts the energy losses function as the parameter, and evaluates it for the requested time period (with optional subperiods).
For now just the first, basic version. It can be later extended to be smarter - for example automatically use shorter time ranges for the energies where higher losses occur.
For now only synchrotron losses implemented, other will be added later.
We can later add the higher level function that would take the flags which radiative processes should be used, instead of accepting generic energy loss function as the parameter.

agnpy/spectra/spectra.py Outdated Show resolved Hide resolved
agnpy/spectra/spectra.py Outdated Show resolved Hide resolved
agnpy/spectra/spectra.py Outdated Show resolved Hide resolved
agnpy/spectra/spectra.py Outdated Show resolved Hide resolved
agnpy/spectra/spectra.py Outdated Show resolved Hide resolved
agnpy/spectra/spectra.py Outdated Show resolved Hide resolved
agnpy/spectra/spectra.py Outdated Show resolved Hide resolved
agnpy/spectra/spectra.py Outdated Show resolved Hide resolved
agnpy/spectra/spectra.py Outdated Show resolved Hide resolved
agnpy/spectra/spectra.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@jsitarek jsitarek left a comment

Choose a reason for hiding this comment

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

thanks for the PR.
I put some comments

Copy link
Collaborator

@jsitarek jsitarek left a comment

Choose a reason for hiding this comment

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

thanks for the changes, everything looks good to me now.
let's see what @cosimoNigro thinks

@grzegorzbor
Copy link
Contributor Author

Regarding the 2 failing tests for agnpy-fit, they are not related to this PR.
It seems there is some issue with the new version of gammapy 1.2, released on February 29.
I ran these tests locally and they work fine with gammapy 1.1, but fail with gammapy 1.2.
So i expect the master branch to also fail after 29th of Feb.

@jsitarek
Copy link
Collaborator

jsitarek commented May 9, 2024

then I think the simplest solution for the moment would be to fix in the environment file gammapy version to ~1.1.
Actually @cosimoNigro why gammapy is installed via pip and not directly?

@grzegorzbor
Copy link
Contributor Author

Hi @cosimoNigro
I updated the dependencies of the project to the combination of the versions that seem to work fine. Also aligned the environment.yml and setup.py files to have the same dependency specification. Now all tests pass.
Unfortunately the build is failing now on the next step, codecoverage, because the call to the codecov API returns an error "Rate limit reached. Please upload with the Codecov repository upload token to resolve issue". I tried a couple of times and it always ends with the same error, so it doesn't seem to be temporary. According to the codecov documentation https://docs.codecov.com/docs/quick-start you need to create an account on codecov, then generate a token, and then update the CI Test workflow definition with the token. For open source projects it should be free of charge.

@cosimoNigro
Copy link
Owner

cosimoNigro commented Oct 8, 2024

Thanks for the work @grzegorzbor and thanks @jsitarek for the review.
Everything looks good to me. I will take care of the codecov issue.

@cosimoNigro cosimoNigro merged commit bbc1370 into cosimoNigro:master Oct 8, 2024
0 of 2 checks passed
@grzegorzbor grzegorzbor deleted the synchrotron-time-evolution branch October 19, 2024 13:38
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