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

Simplify kernel functions #17

Merged
merged 5 commits into from
Sep 11, 2021
Merged

Conversation

LudwigBoess
Copy link
Owner

No description provided.

@codecov
Copy link

codecov bot commented Sep 10, 2021

Codecov Report

❗ No coverage uploaded for pull request base (roadmap_to_2_0@8d35622). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@                Coverage Diff                 @@
##             roadmap_to_2_0       #17   +/-   ##
==================================================
  Coverage                  ?   100.00%           
==================================================
  Files                     ?         7           
  Lines                     ?       236           
  Branches                  ?         0           
==================================================
  Hits                      ?       236           
  Misses                    ?         0           
  Partials                  ?         0           
Flag Coverage Δ
unittests 100.00% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.


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 8d35622...9ff66df. Read the comment docs.

Copy link

@AhmedSalih3d AhmedSalih3d left a comment

Choose a reason for hiding this comment

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

I approve these changes I see that you have implemented it as we have been talking about.

Kernels have no neighbour value any more, and dim is used to control dimensions with 3 being the default. Allows for easy construction of kernels and defining their derivatives when related to h^dim atleast.

You also updated all the tests I see, which looks good.

A minor note is, the way of writing W you have chosen does not show in the default Julia terminal. Perhaps this should be changed?

image

Copy link

@AhmedSalih3d AhmedSalih3d left a comment

Choose a reason for hiding this comment

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

Had a look through the explanation files, looks fine

@LudwigBoess
Copy link
Owner Author

I can't reproduce the unicode error you see with 𝒲. For me it works fine in the Julia REPL.
Can you try typing it out like it says with \scrW and then tab complete?

@AhmedSalih3d
Copy link

I can't reproduce the unicode error you see with 𝒲. For me it works fine in the Julia REPL.
Can you try typing it out like it says with \scrW and then tab complete?

image

Seems to not work for me :)

Using Julia 1.5 with no fancy stuff, perhaps there is a error in my install.

Kind regards

@LudwigBoess
Copy link
Owner Author

Ah, maybe that's one of the new utf-8 characters they allow in 1.6.
I use 1.6 and there it works fine.
If it's ok for you I'll just leave it as is...

@AhmedSalih3d
Copy link

Ah, maybe that's one of the new utf-8 characters they allow in 1.6.
I use 1.6 and there it works fine.
If it's ok for you I'll just leave it as is...

Since we know the reason, totally fine - keep it as is!

@LudwigBoess LudwigBoess merged commit dfd5ecb into roadmap_to_2_0 Sep 11, 2021
@LudwigBoess LudwigBoess deleted the simplify_kernel_functions branch September 11, 2021 13:06
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.

2 participants