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

Incorrect rendering of piecewise constant pseudocolor apears in VisIt 3.4.1 #19469

Open
sethwatts opened this issue Apr 25, 2024 · 15 comments
Open
Assignees
Labels
bug Something isn't working impact medium Productivity partially degraded (not easily mitigated bug) or improved (enhancement) likelihood medium Neither low nor high likelihood reviewed Issue has been reviewed and labeled by a developer
Milestone

Comments

@sethwatts
Copy link

Hello -

I represent a team that solves topology optimization and related problems, so we frequently wish to render element-wise constant fields on meshes of various geometries. I recently heard from two team members that the default visit installation on LC (at /usr/gapps/visit/bin/visit, which is now visit 3.4.1) now renders such fields incorrectly, as depicted below, and also yields an error message. This behavior is new relative to visit 3.3, which we had been using previously (are now using again to correctly render the fields).

Here is a representation of the issue. The two images below are renderings of the same data on the same mesh, saved from MFEM as an mfem::VisitDataCollection. The only difference is that the first image was rendered using visit 3.3, and the second one was rendered using visit 3.4. The 3.3 version is piecewise constant over the elements as expected, and moreover has the expected radially symmetry (expected due to constraints in the design data before being saved). The 3.4 version is not radially symmetric, nor is the field constant over the elements - it looks, at least in places, like a C0-continuous, second order field.

image

image

Additionally, the 3.4 version issues the following error message:

image

which seems like it may be consistent with an expectation of a nodal (vice elementwise) field.

Another team member has seen the same effect on other meshes (which I can't share here), leading me to suspect the issue is not specific to the mesh shown here. I will ask the team member that produced the images above to provide a tarball of the data as well as a session file from visit 3.4 to reproduce the unexpected behavior.

@sethwatts sethwatts added bug Something isn't working impact medium Productivity partially degraded (not easily mitigated bug) or improved (enhancement) likelihood medium Neither low nor high likelihood labels Apr 25, 2024
@JustinPrivitera
Copy link
Member

Thanks for your report! That is concerning. If you are not doing so already, I would recommend using visit -v 3.3.3 for your needs. If you could provide us a reproducer that would be very helpful. If you need to provide via LC let me know and I can give you my username.

@sethwatts
Copy link
Author

I am attaching a compressed tarball that contains the data set used to generate the images above. I tried opening visit 3.4 with the session files also included in the tarball and they didn't (seem to?) work for me (although I have not used session files before. I can reproduce the images above, though, by creating a pseudocolor on the scalar field in the sol_CAST_Conf1_TracBC_3_DispBC_2_fricCoeff0p20_*.mfem_root files. Please let me know if reproduction is unclear.

  • Update: the compressed tarball is 38 MB, and apparently I can only upload a file of 25 MB or smaller. If you send me your LC user name, I can give it to you there.

@JustinPrivitera
Copy link
Member

Sorry for the delay; I was on travel last week. My LC username is justin.

@sethwatts
Copy link
Author

Not a problem. I have given you a compressed tarball on quartz.

@JustinPrivitera
Copy link
Member

Thanks; I'll take a look.

@cyrush cyrush added the reviewed Issue has been reviewed and labeled by a developer label Apr 30, 2024
@JustinPrivitera
Copy link
Member

@sethwatts I dug into this issue and found the cause. The short answer is that your windingAngle variable is listed as nodal in all your root files, when it should be zonal:
image
If I change the "assoc" entry to "elements", I see the correct behavior with VisIt 3.4.1. The metadata needs to agree with the actual data in order for VisIt to do its job correctly.

Why did this work in 3.3.3? 3.3.3 is using an older LOR algorithm from MFEM, which ends up making all mfem mesh variables nodal. This agrees with what you have in your root files for windingAngle, as it is listed as nodal. It appears correct because the old LOR algorithm creates disconnected meshes, so we don't see any interpolation. The new LOR algorithm was made the default in 3.4.0 and creates connected meshes.

You can change the LOR algorithm being used for MFEM with the default open options if you'd prefer to use the legacy LOR algorithm in 3.4.1. I can explain that more if that isn't clear.

I'm going to close this ticket since the issue is outside of VisIt. If you have further questions or comments, feel free to reopen or make a new one.

@sethwatts
Copy link
Author

Thanks, Justin! If I understand your finding correctly, it sounds like MFEM needs to update how it writes its mfem::VisItDataCollection when we save element-wise fields on the mesh (L2 fields in MFEM vernacular). Is that right? If so, I can work on a fix there.

@JustinPrivitera
Copy link
Member

@sethwatts yes that's right. Whatever tool generates the .mfem_root files should ensure that L2 fields are listed as element-associated.

@JustinPrivitera
Copy link
Member

As per the linked discussions above from MFEM, we may wish to explore other options, like using the basis name to infer the field-association. I don't like this idea, and if MFEM changes what they output for L2 fields to correctly be element-associated then I don't see any reason for this. The backwards compatibility issue is upsetting though, but it seems foolish to bake the wrong answer into how our tools work. Someday we will have to rip the band-aid off, or always keep building on a band-aid.

@JustinPrivitera JustinPrivitera removed the reviewed Issue has been reviewed and labeled by a developer label May 7, 2024
@sethwatts
Copy link
Author

I could see it being sufficient, with documentation so that users know this is an option.

I'm wondering if it would be possible when Visit loads an MFEM data file, if it attempts to load the data with the given association and has an error, if it could also try loading with the other association. If that also fails, the error message displayed to the user could reflect this. If the switched association works, Visit could still raise an error message explaining that it has made the substitution and noting the change in expectations as of version 3.4.0. This would both give users the visualization they (presumably) want and educate them on this issue for the future.

I haven't looked at Visit code, so I don't know how feasible this idea might be to implement, so I just mention it for consideration.

@cyrush
Copy link
Member

cyrush commented May 8, 2024

@sethwatts we can see if it is possible to smooth this out. The warning you did see is a more cryptic version of what you are asking for - but of course we didn't auto switch to a different type of data.

We don't have this issue with MFEM data stored via Conduit Blueprint b/c we have a richer description that includes the full basis name. We may move to that for future versions of base MFEM. We could keep the assoc and it would be ignored when the basis info overrides it, so this would not break 3.3.

@JustinPrivitera
Copy link
Member

@sethwatts can I add the files you attached previously to our tests?

@sethwatts
Copy link
Author

Yes, I don't think there is any problem there.

@sethwatts
Copy link
Author

If the files do not contain expected metadata, I can work with the analyst to generate new files, or provide different files also containing constant element-wise data. I will note, however, that I don't see the basis name in other root files, as for example the contents below of a recently-run LiDO example:

{
  "dsets": {
    "main": {
      "cycle": 58,
      "domains": 2,
      "fields": {
        "Displacement": {
          "path": "EX2_field_data_000058/Displacement.%06d",
          "tags": {
            "assoc": "nodes",
            "comps": "3",
            "lod": "1"
          }
        },
        "Filtered design": {
          "path": "EX2_field_data_000058/Filtered design.%06d",
          "tags": {
            "assoc": "nodes",
            "comps": "1",
            "lod": "1"
          }
        },

... and so on.

@JustinPrivitera
Copy link
Member

If we could add basis name for each field (or some piece of metadata, maybe continuity?) then I could make VisIt check for it and do the right thing. I can attempt to add that in MFEM if you'd like. Just not sure where the basis name lives.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working impact medium Productivity partially degraded (not easily mitigated bug) or improved (enhancement) likelihood medium Neither low nor high likelihood reviewed Issue has been reviewed and labeled by a developer
Projects
None yet
Development

No branches or pull requests

3 participants