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.

Sign up for GitHub

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

Jump to bottom

update src/tutorials/output/jbearing2_3.out #35

Merged
merged 1 commit into from
May 21, 2020
Merged

update src/tutorials/output/jbearing2_3.out #35

merged 1 commit into from
May 21, 2020

Conversation

haplav
Copy link
Collaborator

@haplav haplav commented May 13, 2020

fix #19

@haplav haplav requested a review from jkruzik May 13, 2020 19:03
@haplav haplav self-assigned this May 13, 2020
@haplav haplav force-pushed the haplav/update-output-jbearing2 branch from 2abebb7 to 4cf12eb Compare May 13, 2020 19:10
@haplav
Copy link
Collaborator Author

haplav commented May 13, 2020

@jkruzik I'm not sure whether this output file should be updated or an alt file added. Can it be just out of date? Does it pass for you currently?

@jkruzik
Copy link
Member

jkruzik commented May 14, 2020

The current master works for me with PETSC maint (a4758970) with both Clang (10.0.0) and GCC (9.3.0) on AMD Zen 1st gen and I believe it works fine on Intel Broadwell and Coffee Lake with Clang (I will check it in the evening).

The outputs (which were tested on the 3 architectures mentioned above) did not really change since v3.10 (4d1982e), where there was a big performance regression.

As for the timings, I get:

# Timing summary (actual test time / total CPU time): 
#   tutorials-jbearing2_3: 17.71 sec / 33.84 sec
#   tutorials-jbearing2_6: 8.95 sec / 17.09 sec

Perhaps we can reduce the problem size (say mx=my=30) for tests 3 and 6 to reduce the time required and it may fix the BLMVM instability.

@haplav
Copy link
Collaborator Author

haplav commented May 14, 2020

@jkruzik

Perhaps we can reduce the problem size (say mx=my=30) for tests 3 and 6 to reduce the time required and it may fix the BLMVM instability.

Sounds good, will try.

@haplav haplav force-pushed the haplav/update-output-jbearing2 branch from 4cf12eb to e880e31 Compare May 14, 2020 14:23
@haplav
Copy link
Collaborator Author

haplav commented May 14, 2020

Let me also push a suggestion addressing my previous concern #19 (comment).

@haplav haplav force-pushed the haplav/update-output-jbearing2 branch from e880e31 to e8b1b91 Compare May 14, 2020 15:26
@haplav
Copy link
Collaborator Author

haplav commented May 14, 2020

OK, @jkruzik, tested on my end and ready for your review!

jkruzik
jkruzik reviewed May 15, 2020
View reviewed changes
src/tutorials/jbearing2.c Outdated Show resolved Hide resolved
src/tutorials/jbearing2.c Outdated Show resolved Hide resolved
src/tutorials/jbearing2.c Outdated Show resolved Hide resolved
@haplav haplav force-pushed the haplav/update-output-jbearing2 branch from 37ccade to e2bcc13 Compare May 15, 2020 09:45
@haplav haplav mentioned this pull request May 18, 2020
Merged
@haplav haplav force-pushed the haplav/update-output-jbearing2 branch from 1e6911b to cb2ee00 Compare May 18, 2020 15:50
@haplav
Copy link
Collaborator Author

haplav commented May 18, 2020

(rebase onto master)

@haplav
Copy link
Collaborator Author

haplav commented May 18, 2020

OK, @jkruzik, this seems to work as it is now. I suggest merging this (after squashing) and do the things of from #35 (comment) separately once #40 is merged.

@haplav haplav mentioned this pull request May 20, 2020
Open
@haplav
Copy link
Collaborator Author

haplav commented May 20, 2020

@jkruzik I think we are getting there 😅 If you approve, I will rebase, collapse, re-test, merge.

@haplav haplav force-pushed the haplav/update-output-jbearing2 branch from c8dfc7d to 5b73b9a Compare May 21, 2020 09:51
@haplav
Copy link
Collaborator Author

haplav commented May 21, 2020

(pure squash)

@haplav @jkruzik
tutorials-jbearing2 improvements.
6cf836c 
* Reduce problem size and increase relative tolerance to reduce runtime and improve stability.
* rtol = gttol
* Check that TAO and QPS converged.
* Throw error if TAO-PERMON difference is too big.
* Tolerance can be specified with -tao_diff_tol.
* Set TAOTRON as default TaoType.

Co-authored-by: Jakub Kruzik <jakub.kruzik@gmail.com>
@haplav haplav force-pushed the haplav/update-output-jbearing2 branch from 5b73b9a to 6cf836c Compare May 21, 2020 09:52
@haplav
Copy link
Collaborator Author

haplav commented May 21, 2020

(pure rebase)

@haplav
Copy link
Collaborator Author

haplav commented May 21, 2020

Re-tested.

@haplav haplav merged commit 8495190 into master May 21, 2020
@haplav haplav deleted the haplav/update-output-jbearing2 branch May 21, 2020 09:55
@haplav
Copy link
Collaborator Author

haplav commented May 21, 2020

@jkruzik Oops, sorry, I think I confused you approval for #38 :-( I will be more careful next time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jkruzik jkruzik jkruzik left review comments

Assignees

@haplav haplav

Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

tutorials-jbearing2_3 test issue
2 participants
@haplav @jkruzik