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

#261 Improve MC computation to have all procs available running jobs. #262

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

gautierbureau
Copy link
Member

Checklist before requesting a review

use '[x]' to check the checkboxes, or submit the PR and then click the checkboxes

  • unit tests and non regression tests were added (update of algorithms)
  • main documentation was updated (update of input/output file, update of algorithms)
  • if this PR modifies a dictionary: the corresponding french dictionary was updated
  • if this commit targets a release branch: the corresponding milestone was added in the ticket and in this PR

@rosiereflo rosiereflo linked an issue Oct 5, 2022 that may be closed by this pull request
Copy link
Contributor

@rosiereflo rosiereflo left a comment

Choose a reason for hiding this comment

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

Please add a nrt that exercise this

@gautierbureau
Copy link
Member Author

Please add a nrt that exercise this

By design all nrts already exercise this. And no way to check it from an nrt point of view as this behavior is not always active if at some point there is not enough jobs to run...

@rosiereflo
Copy link
Contributor

Please add a nrt that exercise this

By design all nrts already exercise this. And no way to check it from an nrt point of view as this behavior is not always active if at some point there is not enough jobs to run...

the log of the MC should reflect that

@gautierbureau
Copy link
Member Author

Please add a nrt that exercise this

By design all nrts already exercise this. And no way to check it from an nrt point of view as this behavior is not always active if at some point there is not enough jobs to run...

the log of the MC should reflect that

There are no logs as reference

@rosiereflo
Copy link
Contributor

Please add a nrt that exercise this

By design all nrts already exercise this. And no way to check it from an nrt point of view as this behavior is not always active if at some point there is not enough jobs to run...

the log of the MC should reflect that

There are no logs as reference

No, but it might be the right time to add it

@gautierbureau
Copy link
Member Author

Please add a nrt that exercise this

By design all nrts already exercise this. And no way to check it from an nrt point of view as this behavior is not always active if at some point there is not enough jobs to run...

the log of the MC should reflect that

There are no logs as reference

No, but it might be the right time to add it

I am not sure we have something to compare those two logs...

@rosiereflo
Copy link
Contributor

Please add a nrt that exercise this

By design all nrts already exercise this. And no way to check it from an nrt point of view as this behavior is not always active if at some point there is not enough jobs to run...

the log of the MC should reflect that

There are no logs as reference

No, but it might be the right time to add it

I am not sure we have something to compare those two logs...

The log comparison already exists in dynawo, the only things to do is to add the logs in reference and modify the filter in the LineToCompare function in nrtDiff.py

@gautierbureau
Copy link
Member Author

Please add a nrt that exercise this

By design all nrts already exercise this. And no way to check it from an nrt point of view as this behavior is not always active if at some point there is not enough jobs to run...

the log of the MC should reflect that

There are no logs as reference

No, but it might be the right time to add it

I am not sure we have something to compare those two logs...

The log comparison already exists in dynawo, the only things to do is to add the logs in reference and modify the filter in the LineToCompare function in nrtDiff.py

I will add the logs file prior to this PR to see a difference. #308

@rosiereflo
Copy link
Contributor

It seems that the nrt are passing even without updating the logs which is surprising

@gautierbureau
Copy link
Member Author

It seems that the nrt are passing even without updating the logs which is surprising

I think we don't use enough procs to activate the behavior...

@rosiereflo
Copy link
Contributor

It seems that the nrt are passing even without updating the logs which is surprising

I think we don't use enough procs to activate the behavior...

I think we use 2, we need a test of this behavior somehow to avoid silent regressions

@gautierbureau
Copy link
Member Author

I agree

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve MC to have all processors buisy
2 participants