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

Final changes for glvis-4.3 #284

Merged
merged 18 commits into from
Aug 7, 2024
Merged

Final changes for glvis-4.3 #284

merged 18 commits into from
Aug 7, 2024

Conversation

tzanio
Copy link
Member

@tzanio tzanio commented Jun 19, 2024

📆 Target release date: July 15, 2024 August 7, 2024

🟡 In Progress:

🟢 DONE:

After the release:

Postponed:

@tzanio tzanio requested a review from a team June 19, 2024 00:58
@tzanio tzanio self-assigned this Jun 19, 2024
@tzanio tzanio added the release label Jun 19, 2024
@tzanio tzanio added this to the glvis-4.3 milestone Jun 19, 2024
@tzanio tzanio mentioned this pull request Jun 19, 2024
Copy link

@acfisher acfisher left a comment

Choose a reason for hiding this comment

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

LGTM

@najlkin
Copy link
Contributor

najlkin commented Jun 24, 2024

What is the idea with quadrature functions? Interpreting them as L2 elements would be very simple 😉 . I have done that in the past in my codes.

@tzanio
Copy link
Member Author

tzanio commented Jun 24, 2024

What is the idea with quadrature functions? Interpreting them as L2 elements would be very simple 😉 . I have done that in the past in my codes.

This has been discussed multiple times, but there are essentially 3 options we have considered:

  1. Plot p.w. const function on the LOR Gauss-Lobatto mesh, where each LOR element contains exactly one Gauss-Legendre quadrature point. (The p points of 1D GLL quadrature rule are interleaved by the p+1 points of the 1D GL rule)

  2. Interpret quadrature values as the dofs of an L2 grid funcion

  3. Draw a little sphere for each point

Option 3 is most general and will be required for "Support for visualization of particle data". I personally like option 1, because it does not introduce variations beyond the original data. That works only on quad/hex meshes though.

@najlkin
Copy link
Contributor

najlkin commented Jun 24, 2024

I see, we must reach consensus then what way to go, which might take longer than actually implementing it 😁 .

@tzanio
Copy link
Member Author

tzanio commented Jun 24, 2024

We can also support both options 😁

@najlkin najlkin self-assigned this Jul 1, 2024
@najlkin
Copy link
Contributor

najlkin commented Jul 8, 2024

I found out there is no smooth vizualization in 2D for vector fields atm. This could be also added, but it would require to differentiate the vector-to-scalar function 🤔

@najlkin najlkin mentioned this pull request Jul 9, 2024
5 tasks
@tzanio tzanio mentioned this pull request Jul 23, 2024
@justinlaughlin
Copy link
Contributor

In the future should PRs be pulled into glvis-4.x-dev first instead of master?

@tzanio
Copy link
Member Author

tzanio commented Jul 24, 2024

In the future should PRs be pulled into glvis-4.x-dev first instead of master?

No, the current policy is that PRs should go first in master and then we merge master in glvis-4.3-dev.

(The intention for the release branches was to be small and short-lived)

@tzanio
Copy link
Member Author

tzanio commented Jul 25, 2024

MFEM autotest was giving styling errors, so I run "make style" here but the results may not be fully satisfactory.

Please review c697e0b

@najlkin
Copy link
Contributor

najlkin commented Jul 25, 2024

We should really add make style to PR CIs as #288 was merged without it 🙃

@justinlaughlin
Copy link
Contributor

We should really add make style to PR CIs as #288 was merged without it 🙃

Oops, my bad 😅

lib/vssolution.cpp Outdated Show resolved Hide resolved
@tzanio
Copy link
Member Author

tzanio commented Aug 5, 2024

Should we add a test stream for quadrature function visualization?

For example quadrature.saved.zip corresponding to GLVis/web#12 (comment)?

@najlkin
Copy link
Contributor

najlkin commented Aug 5, 2024

Should we add a test stream for quadrature function visualization?

That would be wise 😉 , but we do not have a matching example code for that, but I do not know if it is really a rule to have one... 🤔

@tzanio
Copy link
Member Author

tzanio commented Aug 5, 2024

we do not have a matching example code for that, but I do not know if it is really a rule to have one... 🤔

it is not a hard requirement

Copy link
Contributor

@najlkin najlkin left a comment

Choose a reason for hiding this comment

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

Everything looks good, the quadrature test would be nice if it is added, but it goes to data anyway.

@tzanio
Copy link
Member Author

tzanio commented Aug 6, 2024

the quadrature test would be nice if it is added, but it goes to data anyway.

I don't think it hurts to do it now. I can propose a PR with the above file, unless you prefer something else @najlkin

@najlkin
Copy link
Contributor

najlkin commented Aug 6, 2024

the quadrature test would be nice if it is added, but it goes to data anyway.

I don't think it hurts to do it now. I can propose a PR with the above file, unless you prefer something else @najlkin

No, let us do it 🚀 .

@justinlaughlin
Copy link
Contributor

While we have all this glvis momentum can we get eyes on GLVis/pyglvis#44 so we can merge :). This can be after the glvis release of course. There will be other changes needed for pyglvis (such as updating the glvis-js version hosted on npm), but it will be nice to test with the rewrite.

@tzanio tzanio merged commit da0d756 into master Aug 7, 2024
10 checks passed
@tzanio tzanio deleted the glvis-4.3-dev branch August 7, 2024 17:25
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.

4 participants