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

Update research papers notebook and update the docs #2794

Merged
merged 14 commits into from
Aug 19, 2024

Conversation

KasukabeDefenceForce
Copy link
Contributor

@KasukabeDefenceForce KasukabeDefenceForce commented Aug 13, 2024

📝 Description

Type: 🪲 bugfix | 🚀 feature | ☣️ breaking change | 🚦 testing | 📝 documentation | 🎢 infrastructure

This PR aims at improving the logic for fetching the research papers which use Tardis and updating them in the docs.
Old Logic:
We used to search for "Tardis" and "Kerzendorf" or "tardis" and "supernova" in the full text of the article and were published in the NATUR publication. The total number of articles with this logic were 50.
image
New Logic:
We fetch all the papers which have cited this article. The total number of articles fetched with this logic are 132. You can have a look at these here

📌 Resources

Examples, notebooks, and links to useful references.

🚦 Testing

How did you test these changes?

  • Testing pipeline
  • Other method (describe)
  • My changes can't be tested (explain why)

☑️ Checklist

  • I requested two reviewers for this pull request
  • I updated the documentation according to my changes
  • I built the documentation by applying the build_docs label

Note: If you are not allowed to perform any of these actions, ping (@) a contributor.

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@tardis-bot
Copy link
Contributor

tardis-bot commented Aug 13, 2024

*beep* *bop*
Hi human,
I ran ruff on the latest commit (bf40f94).
Here are the outputs produced.
Results can also be downloaded as artifacts here.
Summarised output:

12	W291   	[*] Trailing whitespace
8	W293   	[*] Blank line contains whitespace
6	E999   	[ ] SyntaxError: Expected ',', found ':'
3	F631   	[ ] Assert test is a non-empty tuple, which is always `True`
2	S113   	[ ] Probable use of requests call without timeout
2	B018   	[ ] Found useless expression. Either assign it to a variable or remove it.
2	W292   	[*] No newline at end of file
1	ANN205 	[ ] Missing return type annotation for staticmethod `get_relative_path`
1	COM818 	[ ] Trailing comma on bare tuple prohibited
1	RET506 	[ ] Unnecessary `else` after `raise` statement
1	I001   	[*] Import block is un-sorted or un-formatted
1	PLW0127	[ ] Self-assignment of variable `observed_spectrum`

Complete output(might be large):

.github/workflows/tardis-research-papers.yml:3:4: E999 SyntaxError: Expected an expression
.github/workflows/tardis-research-papers.yml:28:1: W293 [*] Blank line contains whitespace
.github/workflows/tardis-research-papers.yml:33:1: W293 [*] Blank line contains whitespace
.github/workflows/tardis-research-papers.yml:39:1: W293 [*] Blank line contains whitespace
.github/workflows/tardis-research-papers.yml:57:1: W293 [*] Blank line contains whitespace
.github/workflows/tardis-research-papers.yml:70:1: W293 [*] Blank line contains whitespace
.github/workflows/tardis-research-papers.yml:72:1: W293 [*] Blank line contains whitespace
.github/workflows/tardis-research-papers.yml:74:1: W293 [*] Blank line contains whitespace
.github/workflows/tardis-research-papers.yml:88:1: W293 [*] Blank line contains whitespace
.github/workflows/tardis-research-papers.yml:116:82: W292 [*] No newline at end of file
.zenodo.json:1:1: B018 Found useless expression. Either assign it to a variable or remove it.
.zenodo.json:354:2: W292 [*] No newline at end of file
CHANGELOG.md:4:15: E999 SyntaxError: Expected ',', found ':'
CITATION.cff:3:1: E999 SyntaxError: Invalid annotated assignment target
README.rst:1:1: E999 SyntaxError: Expected a statement
asv.conf.json:1:1: B018 Found useless expression. Either assign it to a variable or remove it.
benchmarks/benchmark_base.py:29:9: ANN205 Missing return type annotation for staticmethod `get_relative_path`
benchmarks/benchmark_base.py:75:9: RET506 Unnecessary `else` after `raise` statement
docs/contributing/development/running_tests.rst:1:1: E999 SyntaxError: Expected a statement
docs/contributing/development/running_tests.rst:8:90: COM818 Trailing comma on bare tuple prohibited
docs/contributing/development/running_tests.rst:41:42: W291 [*] Trailing whitespace
docs/contributing/development/running_tests.rst:64:112: W291 [*] Trailing whitespace
docs/contributing/development/running_tests.rst:65:124: W291 [*] Trailing whitespace
docs/contributing/development/running_tests.rst:69:92: W291 [*] Trailing whitespace
docs/contributing/development/running_tests.rst:70:107: W291 [*] Trailing whitespace
docs/contributing/development/running_tests.rst:76:102: W291 [*] Trailing whitespace
docs/contributing/development/running_tests.rst:78:116: W291 [*] Trailing whitespace
docs/contributing/development/running_tests.rst:80:124: W291 [*] Trailing whitespace
docs/contributing/development/running_tests.rst:92:62: W291 [*] Trailing whitespace
docs/resources/credits.rst:1:1: E999 SyntaxError: Expected a statement
docs/resources/research_done_using_TARDIS/ads.ipynb:cell 1:1:1: I001 [*] Import block is un-sorted or un-formatted
docs/resources/research_done_using_TARDIS/ads.ipynb:cell 2:2:39: W291 [*] Trailing whitespace
docs/resources/research_done_using_TARDIS/ads.ipynb:cell 2:12:11: S113 Probable use of requests call without timeout
docs/resources/research_done_using_TARDIS/ads.ipynb:cell 2:12:60: W291 [*] Trailing whitespace
docs/resources/research_done_using_TARDIS/ads.ipynb:cell 2:28:23: S113 Probable use of requests call without timeout
docs/resources/research_done_using_TARDIS/ads.ipynb:cell 2:28:76: W291 [*] Trailing whitespace
tardis/visualization/tools/tests/test_sdec_plot.py:343:17: F631 Assert test is a non-empty tuple, which is always `True`
tardis/visualization/tools/tests/test_sdec_plot.py:475:13: F631 Assert test is a non-empty tuple, which is always `True`
tardis/visualization/tools/tests/test_sdec_plot.py:557:13: PLW0127 Self-assignment of variable `observed_spectrum`
tardis/visualization/tools/tests/test_sdec_plot.py:618:13: F631 Assert test is a non-empty tuple, which is always `True`
Found 40 errors.
[*] 23 fixable with the `--fix` option.

@KasukabeDefenceForce
Copy link
Contributor Author

I prefer to use requests over the ads package because the former is much faster. I could fetch all the papers in approx 2.5 minutes using requests while it took approx 7 minutes to do the same using ads.

Copy link

codecov bot commented Aug 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 69.37%. Comparing base (8a317dd) to head (bf40f94).
Report is 8 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2794      +/-   ##
==========================================
- Coverage   69.91%   69.37%   -0.55%     
==========================================
  Files         206      206              
  Lines       15523    15518       -5     
==========================================
- Hits        10853    10765      -88     
- Misses       4670     4753      +83     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@KasukabeDefenceForce
Copy link
Contributor Author

@KasukabeDefenceForce KasukabeDefenceForce marked this pull request as ready for review August 14, 2024 13:22
@tardis-bot
Copy link
Contributor

*beep* *bop*

Hi, human.

The docs workflow has succeeded ✔️

Click here to see your results.

@tardis-bot
Copy link
Contributor

tardis-bot commented Aug 14, 2024

*beep* *bop*
Hi human,
I ran benchmarks as you asked comparing master (a054d74) and the latest commit (bf40f94).
Here are the logs produced by ASV.
Results can also be downloaded as artifacts here.

Significantly changed benchmarks:

| Change   | Before [a054d743] <master>   | After [bf40f942]    |   Ratio | Benchmark (Parameter)                                                                           |
|----------|------------------------------|---------------------|---------|-------------------------------------------------------------------------------------------------|
| -        | 3.01±0ms                     | 2.72±0ms            |     0.9 | opacities_opacity_state.BenchmarkOpacitiesOpacityState.time_opacity_state_initialize('scatter') |

All benchmarks:

Benchmarks that have improved:

| Change   | Before [a054d743] <master>   | After [bf40f942]    |   Ratio | Benchmark (Parameter)                                                                           |
|----------|------------------------------|---------------------|---------|-------------------------------------------------------------------------------------------------|
| -        | 3.01±0ms                     | 2.72±0ms            |     0.9 | opacities_opacity_state.BenchmarkOpacitiesOpacityState.time_opacity_state_initialize('scatter') |

Benchmarks that have stayed the same:

| Change   | Before [a054d743] <master>   | After [bf40f942]    | Ratio   | Benchmark (Parameter)                                                                                                               |
|----------|------------------------------|---------------------|---------|-------------------------------------------------------------------------------------------------------------------------------------|
|          | 41.7±20μs                    | 49.3±20μs           | ~1.18   | transport_montecarlo_interaction.BenchmarkTransportMontecarloInteraction.time_line_emission                                         |
|          | 1.54±0.4μs                   | 1.30±0.3μs          | ~0.84   | transport_geometry_calculate_distances.BenchmarkTransportGeometryCalculateDistances.time_calculate_distance_line                    |
|          | 66.8±30μs                    | 48.1±30μs           | ~0.72   | transport_montecarlo_interaction.BenchmarkTransportMontecarloInteraction.time_line_scatter                                          |
|          | 601±100ns                    | 651±200ns           | 1.08    | opacities_opacity.BenchmarkMontecarloMontecarloNumbaOpacities.time_photoabsorption_opacity_calculation                              |
|          | 2.99±0.8μs                   | 3.20±0.5μs          | 1.07    | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_vpacket_within_shell                              |
|          | 5.81±0.8μs                   | 6.17±0.8μs          | 1.06    | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_vpacket                                           |
|          | 6.92±2μs                     | 7.34±1μs            | 1.06    | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_vpacket_volley                                    |
|          | 531±200ns                    | 551±100ns           | 1.04    | opacities_opacity.BenchmarkMontecarloMontecarloNumbaOpacities.time_compton_opacity_calculation                                      |
|          | 39.3±0s                      | 39.5±0.01s          | 1.01    | run_tardis.BenchmarkRunTardis.time_run_tardis                                                                                       |
|          | 201±0.2ns                    | 203±0.3ns           | 1.01    | spectrum_formal_integral.BenchmarkTransportMontecarloFormalIntegral.time_intensity_black_body                                       |
|          | 1.66±0.01ms                  | 1.67±0.01ms         | 1.01    | transport_montecarlo_main_loop.BenchmarkTransportMontecarloMontecarloMainLoop.time_montecarlo_main_loop                             |
|          | 521±200ns                    | 521±100ns           | 1.00    | opacities_opacity.BenchmarkMontecarloMontecarloNumbaOpacities.time_pair_creation_opacity_calculation                                |
|          | 3.71±0.01ms                  | 3.71±0.01ms         | 1.00    | opacities_opacity_state.BenchmarkOpacitiesOpacityState.time_opacity_state_initialize('macroatom')                                   |
|          | 1.05±0m                      | 1.05±0m             | 1.00    | run_tardis.BenchmarkRunTardis.time_run_tardis_rpacket_tracking                                                                      |
|          | 2.07±0m                      | 2.07±0m             | 1.00    | spectrum_formal_integral.BenchmarkTransportMontecarloFormalIntegral.time_FormalIntegrator_functions                                 |
|          | 62.7±0.2ms                   | 62.6±0.02ms         | 1.00    | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_rpacket_trackers_to_dataframe                  |
|          | 756±0.3ns                    | 745±0.3ns           | 0.98    | transport_montecarlo_interaction.BenchmarkTransportMontecarloInteraction.time_thomson_scatter                                       |
|          | 23.4±6μs                     | 23.0±6μs            | 0.98    | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_generate_rpacket_last_interaction_tracker_list |
|          | 30.5±0.05μs                  | 29.9±0.02μs         | 0.98    | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_generate_rpacket_tracker_list                  |
|          | 2.04±2μs                     | 1.96±2μs            | 0.96    | transport_montecarlo_estimators_radfield_estimator_calcs.BenchmarkMontecarloMontecarloNumbaPacket.time_update_line_estimators       |
|          | 2.59±0.4ms                   | 2.49±0.4ms          | 0.96    | transport_montecarlo_single_packet_loop.BenchmarkTransportMontecarloSinglePacketLoop.time_single_packet_loop                        |
|          | 1.25±0μs                     | 1.19±0μs            | 0.95    | transport_geometry_calculate_distances.BenchmarkTransportGeometryCalculateDistances.time_calculate_distance_boundary                |
|          | 2.92±0.6μs                   | 2.76±0.3μs          | 0.95    | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_bad_vpacket                                       |

If you want to see the graph of the results, you can check it here

Comment on lines 84 to 102
- name: Approve pull request (I)
run: gh pr review ${{ steps.create-pr.outputs.pull-request-number }} --approve
env:
GITHUB_TOKEN: ${{ secrets.INFRASTRUCTURE_COORDINATOR_TOKEN }}
if: steps.create-pr.outputs.pull-request-operation == 'created'

- name: Approve pull request (II)
run: gh pr review ${{ steps.create-pr.outputs.pull-request-number }} --approve
env:
GITHUB_TOKEN: ${{ secrets.CORE_COORDINATOR_TOKEN }}
if: steps.create-pr.outputs.pull-request-operation == 'created'

- name: Enable automerge
uses: peter-evans/enable-pull-request-automerge@v2
with:
token: ${{ secrets.INFRASTRUCTURE_COORDINATOR_TOKEN }}
pull-request-number: ${{ steps.create-pr.outputs.pull-request-number }}
merge-method: squash
if: steps.create-pr.outputs.pull-request-operation == 'created'
Copy link
Member

Choose a reason for hiding this comment

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

Are we sure we need this?

@wkerzendorf wkerzendorf merged commit 50cafa5 into tardis-sn:master Aug 19, 2024
15 of 17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants