Enable dt estimate for quads/hexes, extend tests for it - #338

Open
inducer wants to merge 6 commits into
mainfrom
add-tpe-dt-util
Open

Enable dt estimate for quads/hexes, extend tests for it#338
inducer wants to merge 6 commits into
mainfrom
add-tpe-dt-util

Conversation

@inducer

@inducerinducer commented Apr 26, 2024•
edited
Loading

Copy link
Copy Markdown
Owner

@inducerinducer changed the title Enable dt estimate for Quads/Hexes, extend tests for it.Enable dt estimate for Quads/Hexes, extend tests for itApr 26, 2024
@inducerinducer mentioned this pull request Apr 26, 2024
Closed
@MTCam@inducer
Enable dt estimate for quads/hexes, extend tests for it
b3eff8b
Co-authored-by: Andreas Kloeckner <inform@tiker.net>
@inducer

inducer commented Apr 26, 2024

Copy link
Copy Markdown
OwnerAuthor

Well... for some very innocuous-looking changes, this has some very exciting-looking failures. 😬

@MTCam

MTCam commented Apr 26, 2024

Copy link
Copy Markdown
Collaborator

Well... for some very innocuous-looking changes, this has some very exciting-looking failures. 😬

I forgot about these. At some point I had worked out a plan for how to fix these tests, but now have turned away too long to remember. I'll have another look.

@inducerinducer mentioned this pull request Apr 29, 2024
Merged
@inducerinducer changed the title Enable dt estimate for Quads/Hexes, extend tests for itEnable dt estimate for quads/hexes, extend tests for itApr 29, 2024
inducer
inducer commented Apr 29, 2024
View reviewed changes
Comment threadgrudge/dt_utils.py Outdated
volm_discr = dcoll.discr_from_dd(dd)

r_fac = dcoll.dim
if any(not isinstance(grp, SimplexElementGroupBase)

@inducerinducerApr 29, 2024

Copy link
Copy Markdown
OwnerAuthor

Choose a reason for hiding this comment

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

Should check for supported element types instead.

@inducer

inducer commented May 1, 2024

Copy link
Copy Markdown
OwnerAuthor

@MTCam Now all the remaining failures are numerical in nature, illustrating (I think) that this does not find a valid time step estimate for tensor product elements even for wave.

@MTCam

MTCam commented May 1, 2024

Copy link
Copy Markdown
Collaborator

@MTCam Now all the remaining failures are numerical in nature, illustrating (I think) that this does not find a valid time step estimate for tensor product elements even for wave.

If we adjust the sketchy fudge factor in the wave operator's rk4 estimate to 0.23 for TPE. I think it passes. That fudge factor is dependent on element type.

I think I also added a skip for the dim=1 tests for TPEs as there is no 1D TPE.

inducer
inducer commented Jul 26, 2024
View reviewed changes
Comment threadgrudge/dt_utils.py
Comment on lines +246 to +259
For non-simplicial elements, we use the following formula for a mean
cell size measure:

.. math::

r_D = \frac{2~d~V}{\sum_{i=1}^{N_{faces}} F_i},

where :math:`d` is the topological dimension, :math:`V` is the cell volume,
and :math:`F_i` are the areas of each face of the cell. Other valid choices
here include the shortest, longest, average of the cell diagonals, or edges.
The value returned by this routine (i.e. the cell volume divided by the
average cell face area) is bounded by the extrema of the cell edge lengths,
is straightforward to calculate regardless of element shape, and jibes well
with the foregoing calculation for simplicial elements.

@inducerinducerJul 26, 2024

Copy link
Copy Markdown
OwnerAuthor

Choose a reason for hiding this comment

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

@MTCam says this inaccurate.

@MTCamMTCamJul 27, 2024

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Should be fixed up now.

Comment threadtest/test_dt_utils.py Outdated
Comment on lines +158 to +159
@pytest.mark.parametrize("tpe", [False, True])
def test_wave_dt_estimate(actx_factory, dim, degree, tpe, visualize=False):

@inducerinducerJul 26, 2024

Copy link
Copy Markdown
OwnerAuthor

Choose a reason for hiding this comment

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

Also try with anisotropic meshes.

@MTCamMTCamJul 28, 2024

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm attempting to add some gmsh-generated meshes to the tests, but both simplices and tensor product fail when I use those meshes. (3c4ebea)

Comment threadtest/test_dt_utils.py Outdated
Comment on lines +171 to +175
mesh = mgen.generate_regular_rect_mesh(
a=a[:dim], b=b[:dim],
nelements_per_axis=(3,)*dim)
nelements_per_axis=(3,)*dim,
group_cls=group_cls)

@inducerinducerJul 26, 2024

Copy link
Copy Markdown
OwnerAuthor

Choose a reason for hiding this comment

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

Maybe go warped?

@MTCamMTCamJul 28, 2024

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I suspect you won't be super impressed by what I've done in 7b8f37b to get passing warped mesh tests, but please take a look and we can discuss.

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

Reviewers

@MTCamMTCamMTCam left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@inducer@MTCam
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all 
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '

Skip to content
Enable dt estimate for quads/hexes, extend tests for it - #338
Open
inducer wants to merge 6 commits into
mainfrom 
add-tpe-dt-util
Open
Enable dt estimate for quads/hexes, extend tests for it#338
inducer wants to merge 6 commits into
mainfrom 
add-tpe-dt-util
Conversation
@inducer
@inducerinducer
commented
Apr 26, 2024•

edited

Loading

Copy link


Copy Markdown

Owner
@inducerinducer
changed the title
Enable dt estimate for Quads/Hexes, extend tests for it.Enable dt estimate for Quads/Hexes, extend tests for itApr 26, 2024
@inducerinducer
mentioned this pull request
Apr 26, 2024
 Closed

@MTCam@inducer
Enable dt estimate for quads/hexes, extend tests for it
b3eff8b
Co-authored-by: Andreas Kloeckner <inform@tiker.net>
@inducer
inducer
commented
Apr 26, 2024

Copy link


Copy Markdown

OwnerAuthor
Well... for some very innocuous-looking changes, this has some very exciting-looking failures. 😬
@MTCam
MTCam
commented
Apr 26, 2024

Copy link


Copy Markdown

Collaborator
Well... for some very innocuous-looking changes, this has some very exciting-looking failures. 😬
I forgot about these. At some point I had worked out a plan for how to fix these tests, but now have turned away too long to remember. I'll have another look.
@inducerinducer
mentioned this pull request
Apr 29, 2024
 Merged

@inducerinducer
changed the title
Enable dt estimate for Quads/Hexes, extend tests for itEnable dt estimate for quads/hexes, extend tests for itApr 29, 2024
inducer
inducer
commented
Apr 29, 2024
View reviewed changes
Comment threadgrudge/dt_utils.py
Outdated

 volm_discr = dcoll.discr_from_dd(dd)

 r_fac = dcoll.dim
 if any(not isinstance(grp, SimplexElementGroupBase)
@inducerinducerApr 29, 2024

Copy link


Copy Markdown

OwnerAuthor
Choose a reason for hiding this comment

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

Should check for supported element types instead.
@inducer
inducer
commented
May 1, 2024

Copy link


Copy Markdown

OwnerAuthor
@MTCam Now all the remaining failures are numerical in nature, illustrating (I think) that this does not find a valid time step estimate for tensor product elements even for wave.
@MTCam
MTCam
commented
May 1, 2024

Copy link


Copy Markdown

Collaborator
@MTCam Now all the remaining failures are numerical in nature, illustrating (I think) that this does not find a valid time step estimate for tensor product elements even for wave.
If we adjust the sketchy fudge factor in the wave operator's rk4 estimate to 0.23 for TPE. I think it passes. That fudge factor is dependent on element type.
I think I also added a skip for the dim=1 tests for TPEs as there is no 1D TPE.
inducer
inducer
commented
Jul 26, 2024
View reviewed changes
Comment threadgrudge/dt_utils.py

Comment on lines

+246
 to 
+259

 For non-simplicial elements, we use the following formula for a mean
 cell size measure:

 .. math::

 r_D = \frac{2~d~V}{\sum_{i=1}^{N_{faces}} F_i},

 where :math:`d` is the topological dimension, :math:`V` is the cell volume,
 and :math:`F_i` are the areas of each face of the cell. Other valid choices
 here include the shortest, longest, average of the cell diagonals, or edges.
 The value returned by this routine (i.e. the cell volume divided by the
 average cell face area) is bounded by the extrema of the cell edge lengths,
 is straightforward to calculate regardless of element shape, and jibes well
 with the foregoing calculation for simplicial elements.
@inducerinducerJul 26, 2024

Copy link


Copy Markdown

OwnerAuthor
Choose a reason for hiding this comment

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

@MTCam says this inaccurate.
@MTCamMTCamJul 27, 2024

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

Should be fixed up now.
Comment threadtest/test_dt_utils.py
Outdated


Comment on lines

+158
 to 
+159

@pytest.mark.parametrize("tpe", [False, True])
def test_wave_dt_estimate(actx_factory, dim, degree, tpe, visualize=False):
@inducerinducerJul 26, 2024

Copy link


Copy Markdown

OwnerAuthor
Choose a reason for hiding this comment

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

Also try with anisotropic meshes.
@MTCamMTCamJul 28, 2024

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

I'm attempting to add some gmsh-generated meshes to the tests, but both simplices and tensor product fail when I use those meshes. (3c4ebea)
Comment threadtest/test_dt_utils.py
Outdated


Comment on lines

+171
 to 
+175

 mesh = mgen.generate_regular_rect_mesh(
 a=a[:dim], b=b[:dim],
 nelements_per_axis=(3,)*dim)
 nelements_per_axis=(3,)*dim,
 group_cls=group_cls)

@inducerinducerJul 26, 2024

Copy link


Copy Markdown

OwnerAuthor
Choose a reason for hiding this comment

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

Maybe go warped?
@MTCamMTCamJul 28, 2024

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

I suspect you won't be super impressed by what I've done in 7b8f37b to get passing warped mesh tests, but please take a look and we can discuss.
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@MTCamMTCamMTCam left review comments

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

2 participants

@inducer@MTCam

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix");
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Enable dt estimate for quads/hexes, extend tests for it - #338
Open
inducer wants to merge 6 commits into
mainfrom 
add-tpe-dt-util
Open
Enable dt estimate for quads/hexes, extend tests for it#338
inducer wants to merge 6 commits into
mainfrom 
add-tpe-dt-util
Conversation
@inducer
@inducerinducer
commented
Apr 26, 2024•

edited

Loading

Copy link


Copy Markdown

Owner
@inducerinducer
changed the title
Enable dt estimate for Quads/Hexes, extend tests for it.Enable dt estimate for Quads/Hexes, extend tests for itApr 26, 2024
@inducerinducer
mentioned this pull request
Apr 26, 2024
 Closed

@MTCam@inducer
Enable dt estimate for quads/hexes, extend tests for it
b3eff8b
Co-authored-by: Andreas Kloeckner <inform@tiker.net>
@inducer
inducer
commented
Apr 26, 2024

Copy link


Copy Markdown

OwnerAuthor
Well... for some very innocuous-looking changes, this has some very exciting-looking failures. 😬
@MTCam
MTCam
commented
Apr 26, 2024

Copy link


Copy Markdown

Collaborator
Well... for some very innocuous-looking changes, this has some very exciting-looking failures. 😬
I forgot about these. At some point I had worked out a plan for how to fix these tests, but now have turned away too long to remember. I'll have another look.
@inducerinducer
mentioned this pull request
Apr 29, 2024
 Merged

@inducerinducer
changed the title
Enable dt estimate for Quads/Hexes, extend tests for itEnable dt estimate for quads/hexes, extend tests for itApr 29, 2024
inducer
inducer
commented
Apr 29, 2024
View reviewed changes
Comment threadgrudge/dt_utils.py
Outdated

 volm_discr = dcoll.discr_from_dd(dd)

 r_fac = dcoll.dim
 if any(not isinstance(grp, SimplexElementGroupBase)
@inducerinducerApr 29, 2024

Copy link


Copy Markdown

OwnerAuthor
Choose a reason for hiding this comment

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

Should check for supported element types instead.
@inducer
inducer
commented
May 1, 2024

Copy link


Copy Markdown

OwnerAuthor
@MTCam Now all the remaining failures are numerical in nature, illustrating (I think) that this does not find a valid time step estimate for tensor product elements even for wave.
@MTCam
MTCam
commented
May 1, 2024

Copy link


Copy Markdown

Collaborator
@MTCam Now all the remaining failures are numerical in nature, illustrating (I think) that this does not find a valid time step estimate for tensor product elements even for wave.
If we adjust the sketchy fudge factor in the wave operator's rk4 estimate to 0.23 for TPE. I think it passes. That fudge factor is dependent on element type.
I think I also added a skip for the dim=1 tests for TPEs as there is no 1D TPE.
inducer
inducer
commented
Jul 26, 2024
View reviewed changes
Comment threadgrudge/dt_utils.py

Comment on lines

+246
 to 
+259

 For non-simplicial elements, we use the following formula for a mean
 cell size measure:

 .. math::

 r_D = \frac{2~d~V}{\sum_{i=1}^{N_{faces}} F_i},

 where :math:`d` is the topological dimension, :math:`V` is the cell volume,
 and :math:`F_i` are the areas of each face of the cell. Other valid choices
 here include the shortest, longest, average of the cell diagonals, or edges.
 The value returned by this routine (i.e. the cell volume divided by the
 average cell face area) is bounded by the extrema of the cell edge lengths,
 is straightforward to calculate regardless of element shape, and jibes well
 with the foregoing calculation for simplicial elements.
@inducerinducerJul 26, 2024

Copy link


Copy Markdown

OwnerAuthor
Choose a reason for hiding this comment

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

@MTCam says this inaccurate.
@MTCamMTCamJul 27, 2024

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

Should be fixed up now.
Comment threadtest/test_dt_utils.py
Outdated


Comment on lines

+158
 to 
+159

@pytest.mark.parametrize("tpe", [False, True])
def test_wave_dt_estimate(actx_factory, dim, degree, tpe, visualize=False):
@inducerinducerJul 26, 2024

Copy link


Copy Markdown

OwnerAuthor
Choose a reason for hiding this comment

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

Also try with anisotropic meshes.
@MTCamMTCamJul 28, 2024

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

I'm attempting to add some gmsh-generated meshes to the tests, but both simplices and tensor product fail when I use those meshes. (3c4ebea)
Comment threadtest/test_dt_utils.py
Outdated


Comment on lines

+171
 to 
+175

 mesh = mgen.generate_regular_rect_mesh(
 a=a[:dim], b=b[:dim],
 nelements_per_axis=(3,)*dim)
 nelements_per_axis=(3,)*dim,
 group_cls=group_cls)

@inducerinducerJul 26, 2024

Copy link


Copy Markdown

OwnerAuthor
Choose a reason for hiding this comment

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

Maybe go warped?
@MTCamMTCamJul 28, 2024

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

I suspect you won't be super impressed by what I've done in 7b8f37b to get passing warped mesh tests, but please take a look and we can discuss.
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@MTCamMTCamMTCam left review comments

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

2 participants

@inducer@MTCam

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms");
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Enable dt estimate for quads/hexes, extend tests for it - #338
Open
inducer wants to merge 6 commits into
mainfrom 
add-tpe-dt-util
Open
Enable dt estimate for quads/hexes, extend tests for it#338
inducer wants to merge 6 commits into
mainfrom 
add-tpe-dt-util
Conversation
@inducer
@inducerinducer
commented
Apr 26, 2024•

edited

Loading

Copy link


Copy Markdown

Owner
@inducerinducer
changed the title
Enable dt estimate for Quads/Hexes, extend tests for it.Enable dt estimate for Quads/Hexes, extend tests for itApr 26, 2024
@inducerinducer
mentioned this pull request
Apr 26, 2024
 Closed

@MTCam@inducer
Enable dt estimate for quads/hexes, extend tests for it
b3eff8b
Co-authored-by: Andreas Kloeckner <inform@tiker.net>
@inducer
inducer
commented
Apr 26, 2024

Copy link


Copy Markdown

OwnerAuthor
Well... for some very innocuous-looking changes, this has some very exciting-looking failures. 😬
@MTCam
MTCam
commented
Apr 26, 2024

Copy link


Copy Markdown

Collaborator
Well... for some very innocuous-looking changes, this has some very exciting-looking failures. 😬
I forgot about these. At some point I had worked out a plan for how to fix these tests, but now have turned away too long to remember. I'll have another look.
@inducerinducer
mentioned this pull request
Apr 29, 2024
 Merged

@inducerinducer
changed the title
Enable dt estimate for Quads/Hexes, extend tests for itEnable dt estimate for quads/hexes, extend tests for itApr 29, 2024
inducer
inducer
commented
Apr 29, 2024
View reviewed changes
Comment threadgrudge/dt_utils.py
Outdated

 volm_discr = dcoll.discr_from_dd(dd)

 r_fac = dcoll.dim
 if any(not isinstance(grp, SimplexElementGroupBase)
@inducerinducerApr 29, 2024

Copy link


Copy Markdown

OwnerAuthor
Choose a reason for hiding this comment

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

Should check for supported element types instead.
@inducer
inducer
commented
May 1, 2024

Copy link


Copy Markdown

OwnerAuthor
@MTCam Now all the remaining failures are numerical in nature, illustrating (I think) that this does not find a valid time step estimate for tensor product elements even for wave.
@MTCam
MTCam
commented
May 1, 2024

Copy link


Copy Markdown

Collaborator
@MTCam Now all the remaining failures are numerical in nature, illustrating (I think) that this does not find a valid time step estimate for tensor product elements even for wave.
If we adjust the sketchy fudge factor in the wave operator's rk4 estimate to 0.23 for TPE. I think it passes. That fudge factor is dependent on element type.
I think I also added a skip for the dim=1 tests for TPEs as there is no 1D TPE.
inducer
inducer
commented
Jul 26, 2024
View reviewed changes
Comment threadgrudge/dt_utils.py

Comment on lines

+246
 to 
+259

 For non-simplicial elements, we use the following formula for a mean
 cell size measure:

 .. math::

 r_D = \frac{2~d~V}{\sum_{i=1}^{N_{faces}} F_i},

 where :math:`d` is the topological dimension, :math:`V` is the cell volume,
 and :math:`F_i` are the areas of each face of the cell. Other valid choices
 here include the shortest, longest, average of the cell diagonals, or edges.
 The value returned by this routine (i.e. the cell volume divided by the
 average cell face area) is bounded by the extrema of the cell edge lengths,
 is straightforward to calculate regardless of element shape, and jibes well
 with the foregoing calculation for simplicial elements.
@inducerinducerJul 26, 2024

Copy link


Copy Markdown

OwnerAuthor
Choose a reason for hiding this comment

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

@MTCam says this inaccurate.
@MTCamMTCamJul 27, 2024

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

Should be fixed up now.
Comment threadtest/test_dt_utils.py
Outdated


Comment on lines

+158
 to 
+159

@pytest.mark.parametrize("tpe", [False, True])
def test_wave_dt_estimate(actx_factory, dim, degree, tpe, visualize=False):
@inducerinducerJul 26, 2024

Copy link


Copy Markdown

OwnerAuthor
Choose a reason for hiding this comment

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

Also try with anisotropic meshes.
@MTCamMTCamJul 28, 2024

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

I'm attempting to add some gmsh-generated meshes to the tests, but both simplices and tensor product fail when I use those meshes. (3c4ebea)
Comment threadtest/test_dt_utils.py
Outdated


Comment on lines

+171
 to 
+175

 mesh = mgen.generate_regular_rect_mesh(
 a=a[:dim], b=b[:dim],
 nelements_per_axis=(3,)*dim)
 nelements_per_axis=(3,)*dim,
 group_cls=group_cls)

@inducerinducerJul 26, 2024

Copy link


Copy Markdown

OwnerAuthor
Choose a reason for hiding this comment

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

Maybe go warped?
@MTCamMTCamJul 28, 2024

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

I suspect you won't be super impressed by what I've done in 7b8f37b to get passing warped mesh tests, but please take a look and we can discuss.
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@MTCamMTCamMTCam left review comments

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

2 participants

@inducer@MTCam

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links");
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '

Skip to content
Enable dt estimate for quads/hexes, extend tests for it - #338
Open
inducer wants to merge 6 commits into
mainfrom 
add-tpe-dt-util
Open
Enable dt estimate for quads/hexes, extend tests for it#338
inducer wants to merge 6 commits into
mainfrom 
add-tpe-dt-util
Conversation
@inducer
@inducerinducer
commented
Apr 26, 2024•

edited

Loading

Copy link


Copy Markdown

Owner
@inducerinducer
changed the title
Enable dt estimate for Quads/Hexes, extend tests for it.Enable dt estimate for Quads/Hexes, extend tests for itApr 26, 2024
@inducerinducer
mentioned this pull request
Apr 26, 2024
 Closed

@MTCam@inducer
Enable dt estimate for quads/hexes, extend tests for it
b3eff8b
Co-authored-by: Andreas Kloeckner <inform@tiker.net>
@inducer
inducer
commented
Apr 26, 2024

Copy link


Copy Markdown

OwnerAuthor
Well... for some very innocuous-looking changes, this has some very exciting-looking failures. 😬
@MTCam
MTCam
commented
Apr 26, 2024

Copy link


Copy Markdown

Collaborator
Well... for some very innocuous-looking changes, this has some very exciting-looking failures. 😬
I forgot about these. At some point I had worked out a plan for how to fix these tests, but now have turned away too long to remember. I'll have another look.
@inducerinducer
mentioned this pull request
Apr 29, 2024
 Merged

@inducerinducer
changed the title
Enable dt estimate for Quads/Hexes, extend tests for itEnable dt estimate for quads/hexes, extend tests for itApr 29, 2024
inducer
inducer
commented
Apr 29, 2024
View reviewed changes
Comment threadgrudge/dt_utils.py
Outdated

 volm_discr = dcoll.discr_from_dd(dd)

 r_fac = dcoll.dim
 if any(not isinstance(grp, SimplexElementGroupBase)
@inducerinducerApr 29, 2024

Copy link


Copy Markdown

OwnerAuthor
Choose a reason for hiding this comment

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

Should check for supported element types instead.
@inducer
inducer
commented
May 1, 2024

Copy link


Copy Markdown

OwnerAuthor
@MTCam Now all the remaining failures are numerical in nature, illustrating (I think) that this does not find a valid time step estimate for tensor product elements even for wave.
@MTCam
MTCam
commented
May 1, 2024

Copy link


Copy Markdown

Collaborator
@MTCam Now all the remaining failures are numerical in nature, illustrating (I think) that this does not find a valid time step estimate for tensor product elements even for wave.
If we adjust the sketchy fudge factor in the wave operator's rk4 estimate to 0.23 for TPE. I think it passes. That fudge factor is dependent on element type.
I think I also added a skip for the dim=1 tests for TPEs as there is no 1D TPE.
inducer
inducer
commented
Jul 26, 2024
View reviewed changes
Comment threadgrudge/dt_utils.py

Comment on lines

+246
 to 
+259

 For non-simplicial elements, we use the following formula for a mean
 cell size measure:

 .. math::

 r_D = \frac{2~d~V}{\sum_{i=1}^{N_{faces}} F_i},

 where :math:`d` is the topological dimension, :math:`V` is the cell volume,
 and :math:`F_i` are the areas of each face of the cell. Other valid choices
 here include the shortest, longest, average of the cell diagonals, or edges.
 The value returned by this routine (i.e. the cell volume divided by the
 average cell face area) is bounded by the extrema of the cell edge lengths,
 is straightforward to calculate regardless of element shape, and jibes well
 with the foregoing calculation for simplicial elements.
@inducerinducerJul 26, 2024

Copy link


Copy Markdown

OwnerAuthor
Choose a reason for hiding this comment

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

@MTCam says this inaccurate.
@MTCamMTCamJul 27, 2024

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

Should be fixed up now.
Comment threadtest/test_dt_utils.py
Outdated


Comment on lines

+158
 to 
+159

@pytest.mark.parametrize("tpe", [False, True])
def test_wave_dt_estimate(actx_factory, dim, degree, tpe, visualize=False):
@inducerinducerJul 26, 2024

Copy link


Copy Markdown

OwnerAuthor
Choose a reason for hiding this comment

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

Also try with anisotropic meshes.
@MTCamMTCamJul 28, 2024

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

I'm attempting to add some gmsh-generated meshes to the tests, but both simplices and tensor product fail when I use those meshes. (3c4ebea)
Comment threadtest/test_dt_utils.py
Outdated


Comment on lines

+171
 to 
+175

 mesh = mgen.generate_regular_rect_mesh(
 a=a[:dim], b=b[:dim],
 nelements_per_axis=(3,)*dim)
 nelements_per_axis=(3,)*dim,
 group_cls=group_cls)

@inducerinducerJul 26, 2024

Copy link


Copy Markdown

OwnerAuthor
Choose a reason for hiding this comment

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

Maybe go warped?
@MTCamMTCamJul 28, 2024

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

I suspect you won't be super impressed by what I've done in 7b8f37b to get passing warped mesh tests, but please take a look and we can discuss.
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@MTCamMTCamMTCam left review comments

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

2 participants

@inducer@MTCam

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default");
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Enable dt estimate for quads/hexes, extend tests for it - #338
Open
inducer wants to merge 6 commits into
mainfrom 
add-tpe-dt-util
Open
Enable dt estimate for quads/hexes, extend tests for it#338
inducer wants to merge 6 commits into
mainfrom 
add-tpe-dt-util
Conversation
@inducer
@inducerinducer
commented
Apr 26, 2024•

edited

Loading

Copy link


Copy Markdown

Owner
@inducerinducer
changed the title
Enable dt estimate for Quads/Hexes, extend tests for it.Enable dt estimate for Quads/Hexes, extend tests for itApr 26, 2024
@inducerinducer
mentioned this pull request
Apr 26, 2024
 Closed

@MTCam@inducer
Enable dt estimate for quads/hexes, extend tests for it
b3eff8b
Co-authored-by: Andreas Kloeckner <inform@tiker.net>
@inducer
inducer
commented
Apr 26, 2024

Copy link


Copy Markdown

OwnerAuthor
Well... for some very innocuous-looking changes, this has some very exciting-looking failures. 😬
@MTCam
MTCam
commented
Apr 26, 2024

Copy link


Copy Markdown

Collaborator
Well... for some very innocuous-looking changes, this has some very exciting-looking failures. 😬
I forgot about these. At some point I had worked out a plan for how to fix these tests, but now have turned away too long to remember. I'll have another look.
@inducerinducer
mentioned this pull request
Apr 29, 2024
 Merged

@inducerinducer
changed the title
Enable dt estimate for Quads/Hexes, extend tests for itEnable dt estimate for quads/hexes, extend tests for itApr 29, 2024
inducer
inducer
commented
Apr 29, 2024
View reviewed changes
Comment threadgrudge/dt_utils.py
Outdated

 volm_discr = dcoll.discr_from_dd(dd)

 r_fac = dcoll.dim
 if any(not isinstance(grp, SimplexElementGroupBase)
@inducerinducerApr 29, 2024

Copy link


Copy Markdown

OwnerAuthor
Choose a reason for hiding this comment

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

Should check for supported element types instead.
@inducer
inducer
commented
May 1, 2024

Copy link


Copy Markdown

OwnerAuthor
@MTCam Now all the remaining failures are numerical in nature, illustrating (I think) that this does not find a valid time step estimate for tensor product elements even for wave.
@MTCam
MTCam
commented
May 1, 2024

Copy link


Copy Markdown

Collaborator
@MTCam Now all the remaining failures are numerical in nature, illustrating (I think) that this does not find a valid time step estimate for tensor product elements even for wave.
If we adjust the sketchy fudge factor in the wave operator's rk4 estimate to 0.23 for TPE. I think it passes. That fudge factor is dependent on element type.
I think I also added a skip for the dim=1 tests for TPEs as there is no 1D TPE.
inducer
inducer
commented
Jul 26, 2024
View reviewed changes
Comment threadgrudge/dt_utils.py

Comment on lines

+246
 to 
+259

 For non-simplicial elements, we use the following formula for a mean
 cell size measure:

 .. math::

 r_D = \frac{2~d~V}{\sum_{i=1}^{N_{faces}} F_i},

 where :math:`d` is the topological dimension, :math:`V` is the cell volume,
 and :math:`F_i` are the areas of each face of the cell. Other valid choices
 here include the shortest, longest, average of the cell diagonals, or edges.
 The value returned by this routine (i.e. the cell volume divided by the
 average cell face area) is bounded by the extrema of the cell edge lengths,
 is straightforward to calculate regardless of element shape, and jibes well
 with the foregoing calculation for simplicial elements.
@inducerinducerJul 26, 2024

Copy link


Copy Markdown

OwnerAuthor
Choose a reason for hiding this comment

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

@MTCam says this inaccurate.
@MTCamMTCamJul 27, 2024

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

Should be fixed up now.
Comment threadtest/test_dt_utils.py
Outdated


Comment on lines

+158
 to 
+159

@pytest.mark.parametrize("tpe", [False, True])
def test_wave_dt_estimate(actx_factory, dim, degree, tpe, visualize=False):
@inducerinducerJul 26, 2024

Copy link


Copy Markdown

OwnerAuthor
Choose a reason for hiding this comment

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

Also try with anisotropic meshes.
@MTCamMTCamJul 28, 2024

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

I'm attempting to add some gmsh-generated meshes to the tests, but both simplices and tensor product fail when I use those meshes. (3c4ebea)
Comment threadtest/test_dt_utils.py
Outdated


Comment on lines

+171
 to 
+175

 mesh = mgen.generate_regular_rect_mesh(
 a=a[:dim], b=b[:dim],
 nelements_per_axis=(3,)*dim)
 nelements_per_axis=(3,)*dim,
 group_cls=group_cls)

@inducerinducerJul 26, 2024

Copy link


Copy Markdown

OwnerAuthor
Choose a reason for hiding this comment

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

Maybe go warped?
@MTCamMTCamJul 28, 2024

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

I suspect you won't be super impressed by what I've done in 7b8f37b to get passing warped mesh tests, but please take a look and we can discuss.
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@MTCamMTCamMTCam left review comments

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

2 participants

@inducer@MTCam

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers");
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Enable dt estimate for quads/hexes, extend tests for it - #338
Open
inducer wants to merge 6 commits into
mainfrom 
add-tpe-dt-util
Open
Enable dt estimate for quads/hexes, extend tests for it#338
inducer wants to merge 6 commits into
mainfrom 
add-tpe-dt-util
Conversation
@inducer
@inducerinducer
commented
Apr 26, 2024•

edited

Loading

Copy link


Copy Markdown

Owner
@inducerinducer
changed the title
Enable dt estimate for Quads/Hexes, extend tests for it.Enable dt estimate for Quads/Hexes, extend tests for itApr 26, 2024
@inducerinducer
mentioned this pull request
Apr 26, 2024
 Closed

@MTCam@inducer
Enable dt estimate for quads/hexes, extend tests for it
b3eff8b
Co-authored-by: Andreas Kloeckner <inform@tiker.net>
@inducer
inducer
commented
Apr 26, 2024

Copy link


Copy Markdown

OwnerAuthor
Well... for some very innocuous-looking changes, this has some very exciting-looking failures. 😬
@MTCam
MTCam
commented
Apr 26, 2024

Copy link


Copy Markdown

Collaborator
Well... for some very innocuous-looking changes, this has some very exciting-looking failures. 😬
I forgot about these. At some point I had worked out a plan for how to fix these tests, but now have turned away too long to remember. I'll have another look.
@inducerinducer
mentioned this pull request
Apr 29, 2024
 Merged

@inducerinducer
changed the title
Enable dt estimate for Quads/Hexes, extend tests for itEnable dt estimate for quads/hexes, extend tests for itApr 29, 2024
inducer
inducer
commented
Apr 29, 2024
View reviewed changes
Comment threadgrudge/dt_utils.py
Outdated

 volm_discr = dcoll.discr_from_dd(dd)

 r_fac = dcoll.dim
 if any(not isinstance(grp, SimplexElementGroupBase)
@inducerinducerApr 29, 2024

Copy link


Copy Markdown

OwnerAuthor
Choose a reason for hiding this comment

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

Should check for supported element types instead.
@inducer
inducer
commented
May 1, 2024

Copy link


Copy Markdown

OwnerAuthor
@MTCam Now all the remaining failures are numerical in nature, illustrating (I think) that this does not find a valid time step estimate for tensor product elements even for wave.
@MTCam
MTCam
commented
May 1, 2024

Copy link


Copy Markdown

Collaborator
@MTCam Now all the remaining failures are numerical in nature, illustrating (I think) that this does not find a valid time step estimate for tensor product elements even for wave.
If we adjust the sketchy fudge factor in the wave operator's rk4 estimate to 0.23 for TPE. I think it passes. That fudge factor is dependent on element type.
I think I also added a skip for the dim=1 tests for TPEs as there is no 1D TPE.
inducer
inducer
commented
Jul 26, 2024
View reviewed changes
Comment threadgrudge/dt_utils.py

Comment on lines

+246
 to 
+259

 For non-simplicial elements, we use the following formula for a mean
 cell size measure:

 .. math::

 r_D = \frac{2~d~V}{\sum_{i=1}^{N_{faces}} F_i},

 where :math:`d` is the topological dimension, :math:`V` is the cell volume,
 and :math:`F_i` are the areas of each face of the cell. Other valid choices
 here include the shortest, longest, average of the cell diagonals, or edges.
 The value returned by this routine (i.e. the cell volume divided by the
 average cell face area) is bounded by the extrema of the cell edge lengths,
 is straightforward to calculate regardless of element shape, and jibes well
 with the foregoing calculation for simplicial elements.
@inducerinducerJul 26, 2024

Copy link


Copy Markdown

OwnerAuthor
Choose a reason for hiding this comment

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

@MTCam says this inaccurate.
@MTCamMTCamJul 27, 2024

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

Should be fixed up now.
Comment threadtest/test_dt_utils.py
Outdated


Comment on lines

+158
 to 
+159

@pytest.mark.parametrize("tpe", [False, True])
def test_wave_dt_estimate(actx_factory, dim, degree, tpe, visualize=False):
@inducerinducerJul 26, 2024

Copy link


Copy Markdown

OwnerAuthor
Choose a reason for hiding this comment

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

Also try with anisotropic meshes.
@MTCamMTCamJul 28, 2024

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

I'm attempting to add some gmsh-generated meshes to the tests, but both simplices and tensor product fail when I use those meshes. (3c4ebea)
Comment threadtest/test_dt_utils.py
Outdated


Comment on lines

+171
 to 
+175

 mesh = mgen.generate_regular_rect_mesh(
 a=a[:dim], b=b[:dim],
 nelements_per_axis=(3,)*dim)
 nelements_per_axis=(3,)*dim,
 group_cls=group_cls)

@inducerinducerJul 26, 2024

Copy link


Copy Markdown

OwnerAuthor
Choose a reason for hiding this comment

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

Maybe go warped?
@MTCamMTCamJul 28, 2024

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

I suspect you won't be super impressed by what I've done in 7b8f37b to get passing warped mesh tests, but please take a look and we can discuss.
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@MTCamMTCamMTCam left review comments

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

2 participants

@inducer@MTCam

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode");
}
} catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); }
})();
})();

Skip to content
Enable dt estimate for quads/hexes, extend tests for it - #338
Open
inducer wants to merge 6 commits into
mainfrom 
add-tpe-dt-util
Open
Enable dt estimate for quads/hexes, extend tests for it#338
inducer wants to merge 6 commits into
mainfrom 
add-tpe-dt-util
Conversation
@inducer
@inducerinducer
commented
Apr 26, 2024•

edited

Loading

Copy link


Copy Markdown

Owner
@inducerinducer
changed the title
Enable dt estimate for Quads/Hexes, extend tests for it.Enable dt estimate for Quads/Hexes, extend tests for itApr 26, 2024
@inducerinducer
mentioned this pull request
Apr 26, 2024
 Closed

@MTCam@inducer
Enable dt estimate for quads/hexes, extend tests for it
b3eff8b
Co-authored-by: Andreas Kloeckner <inform@tiker.net>
@inducer
inducer
commented
Apr 26, 2024

Copy link


Copy Markdown

OwnerAuthor
Well... for some very innocuous-looking changes, this has some very exciting-looking failures. 😬
@MTCam
MTCam
commented
Apr 26, 2024

Copy link


Copy Markdown

Collaborator
Well... for some very innocuous-looking changes, this has some very exciting-looking failures. 😬
I forgot about these. At some point I had worked out a plan for how to fix these tests, but now have turned away too long to remember. I'll have another look.
@inducerinducer
mentioned this pull request
Apr 29, 2024
 Merged

@inducerinducer
changed the title
Enable dt estimate for Quads/Hexes, extend tests for itEnable dt estimate for quads/hexes, extend tests for itApr 29, 2024
inducer
inducer
commented
Apr 29, 2024
View reviewed changes
Comment threadgrudge/dt_utils.py
Outdated

 volm_discr = dcoll.discr_from_dd(dd)

 r_fac = dcoll.dim
 if any(not isinstance(grp, SimplexElementGroupBase)
@inducerinducerApr 29, 2024

Copy link


Copy Markdown

OwnerAuthor
Choose a reason for hiding this comment

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

Should check for supported element types instead.
@inducer
inducer
commented
May 1, 2024

Copy link


Copy Markdown

OwnerAuthor
@MTCam Now all the remaining failures are numerical in nature, illustrating (I think) that this does not find a valid time step estimate for tensor product elements even for wave.
@MTCam
MTCam
commented
May 1, 2024

Copy link


Copy Markdown

Collaborator
@MTCam Now all the remaining failures are numerical in nature, illustrating (I think) that this does not find a valid time step estimate for tensor product elements even for wave.
If we adjust the sketchy fudge factor in the wave operator's rk4 estimate to 0.23 for TPE. I think it passes. That fudge factor is dependent on element type.
I think I also added a skip for the dim=1 tests for TPEs as there is no 1D TPE.
inducer
inducer
commented
Jul 26, 2024
View reviewed changes
Comment threadgrudge/dt_utils.py

Comment on lines

+246
 to 
+259

 For non-simplicial elements, we use the following formula for a mean
 cell size measure:

 .. math::

 r_D = \frac{2~d~V}{\sum_{i=1}^{N_{faces}} F_i},

 where :math:`d` is the topological dimension, :math:`V` is the cell volume,
 and :math:`F_i` are the areas of each face of the cell. Other valid choices
 here include the shortest, longest, average of the cell diagonals, or edges.
 The value returned by this routine (i.e. the cell volume divided by the
 average cell face area) is bounded by the extrema of the cell edge lengths,
 is straightforward to calculate regardless of element shape, and jibes well
 with the foregoing calculation for simplicial elements.
@inducerinducerJul 26, 2024

Copy link


Copy Markdown

OwnerAuthor
Choose a reason for hiding this comment

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

@MTCam says this inaccurate.
@MTCamMTCamJul 27, 2024

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

Should be fixed up now.
Comment threadtest/test_dt_utils.py
Outdated


Comment on lines

+158
 to 
+159

@pytest.mark.parametrize("tpe", [False, True])
def test_wave_dt_estimate(actx_factory, dim, degree, tpe, visualize=False):
@inducerinducerJul 26, 2024

Copy link


Copy Markdown

OwnerAuthor
Choose a reason for hiding this comment

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

Also try with anisotropic meshes.
@MTCamMTCamJul 28, 2024

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

I'm attempting to add some gmsh-generated meshes to the tests, but both simplices and tensor product fail when I use those meshes. (3c4ebea)
Comment threadtest/test_dt_utils.py
Outdated


Comment on lines

+171
 to 
+175

 mesh = mgen.generate_regular_rect_mesh(
 a=a[:dim], b=b[:dim],
 nelements_per_axis=(3,)*dim)
 nelements_per_axis=(3,)*dim,
 group_cls=group_cls)

@inducerinducerJul 26, 2024

Copy link


Copy Markdown

OwnerAuthor
Choose a reason for hiding this comment

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

Maybe go warped?
@MTCamMTCamJul 28, 2024

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

I suspect you won't be super impressed by what I've done in 7b8f37b to get passing warped mesh tests, but please take a look and we can discuss.
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@MTCamMTCamMTCam left review comments

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

2 participants

@inducer@MTCam