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 PyTest fixtures and updated mainloop to properly compile #1901

Merged
merged 3 commits into from
Feb 23, 2022

Conversation

Rodot-
Copy link
Contributor

@Rodot- Rodot- commented Feb 21, 2022

added in a special fixture for the montecarlo_1e5 model tests as to a void mixing config fixtures. Set the montecarlo_main_loop to recompile itself on the first iteration ot set proper config options

Test for numba 1e5 model was failing when run on it's own but not when run as part of the test suite. This was caused by the single packet seed setting incorrectly setting every packet to the same seed, which was removed. This indicated a larger problem where the montecarlo main loop was not properly being updated with the config changes after subsequent runs, so a change was added to recompile the main loop on the first iteration of each tardis run.

Motivation and context
Tests failing inconsistently

How has this been tested?

  • Testing pipeline.
  • Other.

Type of change

  • Bug fix.
  • New feature.
  • Breaking change.
  • None of the above.

Checklist

  • My change requires a change to the documentation.
    • I have updated the documentation accordingly.
    • (optional) I have built the documentation on my fork following the instructions.
  • I have assigned and requested two reviewers for this pull request.

…void mixing config fixtures. Set the montecarlo_main_loop to recompile itself on the first iteration ot set proper config options
@Rodot- Rodot- requested review from andrewfullard and wkerzendorf and removed request for andrewfullard February 21, 2022 21:10
@tardis-bot
Copy link
Contributor

Before a pull request is accepted, it must meet the following criteria:

  • Is the necessary information provided?
  • Is this a duplicate PR?
    • If a new PR is clearly a duplicate, ask how this PR is different from the original PR?
    • If this PR is about to be merged, close the original PR with a link to this new PR that solved the issue.
  • Does it pass existing tests and are new tests provided if required?
    • The test coverage should not decrease, and for new features should be close to 100%.
  • Is the code tidy?
    • No unnecessary print lines or code comments.

@epassaro
Copy link
Member

/azp run update-refdata

@Rodot-
Copy link
Contributor Author

Rodot- commented Feb 21, 2022

/azp refdata-compare

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@azure-pipelines
Copy link

Command 'refdata-compare' is not supported by Azure Pipelines.

Supported commands
  • help:
    • Get descriptions, examples and documentation about supported commands
    • Example: help "command_name"
  • list:
    • List all pipelines for this repository using a comment.
    • Example: "list"
  • run:
    • Run all pipelines or specific pipelines for this repository using a comment. Use this command by itself to trigger all related pipelines, or specify specific pipelines to run.
    • Example: "run" or "run pipeline_name, pipeline_name, pipeline_name"
  • where:
    • Report back the Azure DevOps orgs that are related to this repository and org
    • Example: "where"

See additional documentation.

@Rodot-
Copy link
Contributor Author

Rodot- commented Feb 21, 2022

/azp run compare-refdata

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@azure-pipelines
Copy link

Build succeeded 0432498

Click here to see results.

@Rodot-
Copy link
Contributor Author

Rodot- commented Feb 22, 2022

/azp run compare-refdata

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@codecov
Copy link

codecov bot commented Feb 22, 2022

Codecov Report

Merging #1901 (3e38d38) into master (69b433c) will increase coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1901   +/-   ##
=======================================
  Coverage   61.17%   61.18%           
=======================================
  Files          69       69           
  Lines        7331     7332    +1     
=======================================
+ Hits         4485     4486    +1     
  Misses       2846     2846           
Impacted Files Coverage Δ
tardis/tardis/montecarlo/montecarlo_numba/base.py 32.74% <0.00%> (+0.60%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 69b433c...3e38d38. Read the comment docs.

@azure-pipelines
Copy link

Build succeeded f954904

Click here to see results.

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.

5 participants