Create a PNG image output object #13646

Hysrok · 2019-06-25T23:35:55Z

Adds the ability to create PNG files from the data provided by input files.
Can specify desired resolution.
Default output is grayscale, but can be put into color (current color scheme 'low -> high' is 'blue->red->yellow->white')
Can specify to save one specific step PNG (default is to save all steps with filename plus numerical identifier).

Closes #12846

permcody

I'm not going to repeat everything in every case. Go ahead and go over this PR and make sure you address each of my comments everywhere where it applies. Let me know if anything doesn't make sense. Nice work!

test/tests/kernels/simple_transient_diffusion/simple_transient_diffusion.i

test/tests/kernels/adv_diff_reaction/adv_diff_reaction_test.i

test/tests/bcs/periodic/wedge.i

test/Makefile

framework/src/outputs/PNGTest.C

dschwen · 2019-06-26T18:41:43Z

How about calling this PNGOutput? "Object" is a little strange.

dschwen · 2019-06-26T18:44:12Z

framework/include/outputs/PNGObject.h

+  PNGObject(const InputParameters & parameters);
+
+  // Method for assigning color values to the PNG
+  void setRGB(png_byte *rgb, float selection);


How is this working? You are not including libpng stuff here but use this type in a public interface. Does it have to be public in the first place?

Probably doesn't need to be public. This PR has a ways to go. I'm helping Sam get the right things into the Makefile and then we'll continue to work on the interface.

Yeah, I don't see a reason any of it should be public except the constructor. Public was just easier to start. Right now I switched everything except the constructor to protected. Not sure if any of it should be private yet. I haven't really thought about what other classes would need from this class, if they'd ever need anything at all.

dschwen · 2019-06-27T17:04:40Z

@Hysrok you might want to look into configuring your editor to automatically run clang-format on save...
Here is what I use: https://www.mooseframework.org/help/development/Atom_Editor.html

permcody

OK - this is my thorough review

framework/include/outputs/PNGOutput.h

framework/src/outputs/PNGOutput.C

test/Makefile

permcody · 2019-06-27T19:44:29Z

test/tests/outputs/png/adv_diff_reaction_test.i

@@ -0,0 +1,124 @@
+[Mesh]


Great! Now you need to add the "tests" file so that these actually get run. I'm going to suggest a "CheckFiles" test but we might be able to do better.

@aeslaughter - Can we do a png difference or is that only available within the python testing infrastructure?

test/tests/outputs/png/adv_diff_reaction_test.i

test/tests/outputs/png/wedge.i

test/tests/outputs/png/adv_diff_reaction_test.i

framework/include/outputs/PNGOutput.h

test/tests/outputs/png/wedge.i

permcody · 2019-06-28T14:43:48Z

@Hysrok - I'm going to recommend that you start your fixes by looking at the non-unity build first. You are missing a lot of headers still...

Rule of thumb, if you use a type in your code, include the header (Prefer the .C file unless you have to put it in the .h file).

If you want to compile a non-unity build locally set MOOSE_UNITY=false in your environment and rebuild everything.

permcody · 2019-06-28T14:53:09Z

@Hysrok - I added one more line to my existing Makefile branch for the include path to the header. We'll need that to make sure we pick up the right version of the png.h file. I think we are getting lucky and picking it up through conda or vtk (it exists in both places).

dschwen · 2019-06-28T23:13:29Z

Just a thought, instead of reinventing the wheel on color output why not use a library like
https://github.com/richardroberts1992/Spectrum
to translate float values into colors taken from a colormap? This might very well be a follow on PR though.

friedmud · 2019-06-29T01:21:19Z

Spectrum does look cool - but why does in include QDebug?

On Fri, Jun 28, 2019 at 5:13 PM Daniel Schwen ***@***.***> wrote: Just a thought, instead of reinventing the wheel on color output why not use a library like https://github.com/richardroberts1992/Spectrum to translate float values into colors taken from a colormap? This might very well be a follow on PR though. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#13646?email_source=notifications&email_token=AAGUSMMNY4P73K2A5X4TUALP42LJTA5CNFSM4H3NEVWKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODY3LPIY#issuecomment-506902435>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAGUSMIPLMVCVE6NZ542LBTP42LJTANCNFSM4H3NEVWA> .

-- Sent from my iPhone

dschwen · 2019-06-29T02:11:32Z

Yeah, weird. Doesn't seem to use it though. Make a PR?

Hysrok · 2019-07-04T01:05:11Z

ssh: connect to host github.com port 22: Operation timed out
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.

I've been getting this problem a lot today. I found a work around that worked for me, but now it looks like even the moosebuild precheck is having the problem. Is something wrong with GitHub, or am I just doing something wrong?

permcody · 2019-07-08T18:21:25Z

If you haven't heard from the MOOSE team. The problem is INL related, not you. They are still working on a fix as of now. You can use HTTPS instead of SSH to get around the issue.

…

On Wed, Jul 3, 2019 at 7:05 PM Herald of Heraldo ***@***.***> wrote: fatal: Could not read from remote repository. Please make sure you have the correct access rights and the repository exists.``` I've been getting this problem a lot today. I found a work around that worked for me, but now it looks like even the moose build checker is having the problem. Is something wrong with GitHub, or am I just doing something wrong? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#13646?email_source=notifications&email_token=AAXFOICC5EF5J6L35NI5XKDP5VEEVA5CNFSM4H3NEVWKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZGBB6A#issuecomment-508301560>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAXFOICA2DJ43F2D7MEFRZTP5VEEVANCNFSM4H3NEVWA> .

framework/src/outputs/PNGOutput.C

permcody · 2019-07-09T16:02:27Z

@Hysrok - Looks like this is failing to build on Linux still. From a quick glance it might be that we are picking up inconsistent headers from libPNG or something. To troubleshoot this, you are going to want to login to a Linux system with the exact same environment as the build systems.

ssh to "rod.hpc.inl.gov"
run "moosebuild"
clone, build libmesh, build moose and start troubleshooting

I can help when you get to step #3. I suspect we have some more work todo to get the include and library flags rock solid.

moosebuild · 2019-07-11T16:57:46Z

Job Precheck on 6e158c4 wanted to post the following:

Your code requires style changes.

A patch was auto generated and copied here
You can directly apply the patch by running, in the top level of your repository:

curl -s https://mooseframework.inl.gov/docs/PRs/13646/style.patch | git apply -v

Alternatively, with your repository up to date and in the top level of your repository:

git clang-format cfcf841ec59b526ac8b1c538a1cb1b50a8811b22

permcody · 2019-07-11T19:13:18Z

Ought oh.... Somehow you have a mess of other peoples commits in here now. Did you forget to rebase?

permcody · 2019-07-11T19:14:27Z

Worse yet you clobbered your change you made earlier just before lunch.

moosebuild · 2019-07-11T20:01:54Z

Job Documentation on 985490b wanted to post the following:

View the site here

This comment will be updated on new commits.

Closes idaholab#12846

- Fixed some formatting errors. - Renamed the classes. - Created tests that don't change the original tests.

refs idaholab#12846

-Using inheriting from FileOutput -Input files for the png's aren't worrying about exodus etc -Changed several types -Removed unnecessary hardcoded lines from MakeFile -Fixed naming convention of variables -Added an #IFDEF to the header file

- Added the header files that were missing. - Removed the macros. - Redid filename configurations for created png's. - Using MooseUtils to check file writability.

…st).

- Added include path to the header to make sure we get the right version of png.h.

- Added vector to store the row data to help avoid raw memory management. - Revmoved the parameter PNGFile and anywhere it was referenced to. - Added possible blue->red color scheme. Commented out for now.

…select a color scheme - Added additional comments to explain the code

-Wrapped PNGOoutput.C in MOOSE_HAVE_LIBPNG wrapper. -Updated some colors. -Added tranparency options.

-Added PNGOutput.md file -Added "design" parameter in test input files -Set max_parallel parameter to 1 for adv_diff_reaction -Lowered the resolution of created images to avoid getting timed out

Removing duplicates and adding detail.

permcody

Looks good! I updated your requirements a bit since we don't accept duplicate requirements anymore. Once this tests successfully I'll get it merged. Let me know when you get in and we can talk about what's next and your poster.

dschwen · 2019-07-16T15:13:33Z

framework/doc/content/source/outputs/png/PNGOutput.md

+  [png]
+    type = PNGOutput
+    transparent = true    #indicates whether the background will be transparent
+    resolution = 50       #resolution of each point


Just looking at the documentation strings I have no idea what this number means. The code suggests it is a scale factor to get from MOOSE length units to pixels. I suggest to make this clear in the docs.

The resolution might be "pixels", except that's not even a great definition with things like retina displays and everything else that complicates rendering sizes. This is the number of "samples" taken in the x and y directions right now but that's not the best solution for resolution, which should be in units per something. Also we need to account for different spacing in x and y. More work to do.

permcody · 2019-07-16T19:02:06Z

@dschwen - We are going to be working on the next phase of this project now. This capability isn't useful yet on anything but small images because of the extensive use of MeshFunction. It can take minutes to generate a single image on a workstation size run right now. I'm going to merge this in and we will continue to work on documentation and capability as we make this scalable.

- PNGOutputter restricted to work in dynaimc linkage mode only due to unresolved linkage problem with the library in static mode - Some constness added to member variables - Memory issue fixed refs idaholab#13646

friedmud · 2019-07-23T00:10:42Z

Hmm - I really think pixels is what we want for resolution... ultimately I want to control the exact size of the PNG.

…

On Tue, Jul 16, 2019 at 1:04 PM Cody Permann ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In framework/doc/content/source/outputs/png/PNGOutput.md <#13646 (comment)>: > @@ -0,0 +1,18 @@ +# PNGOutput (Outputs) + +The PNGOutput object is the class for writing png files from the input files. + +Following is an example of all the different input variables that can be used. + +``` +[Outputs] + [png] + type = PNGOutput + transparent = true #indicates whether the background will be transparent + resolution = 50 #resolution of each point The resolution might be "pixels", except that's not even a great definition with things like retina displays and everything else that complicates rendering sizes. This is the number of "samples" taken in the x and y directions right now but that's not the best solution for resolution, which should be in units per something. Also we need to account for different spacing in x and y. More work to do. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#13646?email_source=notifications&email_token=AAGUSMODS2QS3XYTDOI6WETP7YLTXA5CNFSM4H3NEVWKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB6TU4OQ#discussion_r304072687>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAGUSMP64GSPGWMF4U37VMDP7YLTXANCNFSM4H3NEVWA> .

permcody requested changes Jun 26, 2019

View reviewed changes

dschwen reviewed Jun 26, 2019

View reviewed changes

permcody mentioned this pull request Jun 26, 2019

Adding in optional linking and compiler defines for libPNG #13654

Closed

Hysrok force-pushed the PNGOuput branch from e44b4c3 to 1ca0747 Compare June 27, 2019 16:59

Hysrok force-pushed the PNGOuput branch from 1ca0747 to 76315c2 Compare June 27, 2019 19:13

permcody requested changes Jun 27, 2019

View reviewed changes

dschwen reviewed Jun 27, 2019

View reviewed changes

framework/include/outputs/PNGOutput.h Outdated Show resolved Hide resolved

dschwen reviewed Jun 27, 2019

View reviewed changes

test/tests/outputs/png/wedge.i Outdated Show resolved Hide resolved

dschwen changed the title ~~Created a class to create png files based on the tests that are run.~~ Create a PNG image output object Jun 28, 2019

Hysrok force-pushed the PNGOuput branch 3 times, most recently from b1d1ffc to 49818fb Compare July 4, 2019 00:55

Hysrok force-pushed the PNGOuput branch from 49818fb to 8212b2c Compare July 9, 2019 15:50

permcody reviewed Jul 9, 2019

View reviewed changes

framework/src/outputs/PNGOutput.C Outdated Show resolved Hide resolved

Hysrok force-pushed the PNGOuput branch from 6e158c4 to 5b6f34f Compare July 11, 2019 17:04

Hysrok force-pushed the PNGOuput branch from ef74d24 to cb841c0 Compare July 11, 2019 19:22

Hysrok force-pushed the PNGOuput branch 2 times, most recently from 4488613 to fb059a3 Compare July 12, 2019 18:22

Samuel Tew and others added 12 commits July 12, 2019 14:55

Created a class to create png files based on the tests that are run.

2ee35c2

Closes idaholab#12846

General clean up and fixes to PNGOutput

ccdb50a

- Fixed some formatting errors. - Renamed the classes. - Created tests that don't change the original tests.

Adding in optional linking and compiler defines for libPNG

5d9b689

refs idaholab#12846

Applied clang-format

c7f1ccc

Applied additional changed from pull request comments.

b0fe81e

- Added the header files that were missing. - Removed the macros. - Redid filename configurations for created png's. - Using MooseUtils to check file writability.

Added tests to make sure png's are being printed (files with .png exi…

9382ad1

…st).

Added newest changes from the Makefile branch.

54e8d28

- Added include path to the header to make sure we get the right version of png.h.

Some clean up and additional changes from PR comments.

7881331

- Added vector to store the row data to help avoid raw memory management. - Revmoved the parameter PNGFile and anywhere it was referenced to. - Added possible blue->red color scheme. Commented out for now.

- Added additional color schemes - Added MooseEnum as the new way to …

415aa1e

…select a color scheme - Added additional comments to explain the code

Applied changes.

d9026ad

-Wrapped PNGOoutput.C in MOOSE_HAVE_LIBPNG wrapper. -Updated some colors. -Added tranparency options.

Made changes to fix fails in the moosebuild

a002d0a

-Added PNGOutput.md file -Added "design" parameter in test input files -Set max_parallel parameter to 1 for adv_diff_reaction -Lowered the resolution of created images to avoid getting timed out

Hysrok force-pushed the PNGOuput branch from fb059a3 to a002d0a Compare July 12, 2019 21:00

Updating requirements a bit

e57b086

Removing duplicates and adding detail.

permcody previously approved these changes Jul 16, 2019

View reviewed changes

permcody added the PR: Auto Merge Add this label to have CIVET merge on success label Jul 16, 2019

dschwen reviewed Jul 16, 2019

View reviewed changes

Fixing Adv_Diff_reaction Test spec

985490b

permcody dismissed their stale review via 985490b July 16, 2019 15:55

permcody approved these changes Jul 16, 2019

View reviewed changes

permcody merged commit 5351080 into idaholab:next Jul 16, 2019

permcody mentioned this pull request Jul 17, 2019

Fixing up PNGOutputter issues #13753

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create a PNG image output object #13646

Create a PNG image output object #13646

Hysrok commented Jun 25, 2019

permcody left a comment

dschwen commented Jun 26, 2019

dschwen Jun 26, 2019

permcody Jun 26, 2019

Hysrok Jun 28, 2019

dschwen commented Jun 27, 2019

permcody left a comment

permcody Jun 27, 2019

permcody commented Jun 28, 2019

permcody commented Jun 28, 2019

dschwen commented Jun 28, 2019

friedmud commented Jun 29, 2019 via email

dschwen commented Jun 29, 2019

Hysrok commented Jul 4, 2019 •

edited

permcody commented Jul 8, 2019 via email

permcody commented Jul 9, 2019

moosebuild commented Jul 11, 2019

permcody commented Jul 11, 2019

permcody commented Jul 11, 2019

moosebuild commented Jul 11, 2019 •

edited

permcody left a comment

dschwen Jul 16, 2019

permcody Jul 16, 2019

permcody commented Jul 16, 2019

friedmud commented Jul 23, 2019 via email

Create a PNG image output object #13646

Create a PNG image output object #13646

Conversation

Hysrok commented Jun 25, 2019

permcody left a comment

Choose a reason for hiding this comment

dschwen commented Jun 26, 2019

dschwen Jun 26, 2019

Choose a reason for hiding this comment

permcody Jun 26, 2019

Choose a reason for hiding this comment

Hysrok Jun 28, 2019

Choose a reason for hiding this comment

dschwen commented Jun 27, 2019

permcody left a comment

Choose a reason for hiding this comment

permcody Jun 27, 2019

Choose a reason for hiding this comment

permcody commented Jun 28, 2019

permcody commented Jun 28, 2019

dschwen commented Jun 28, 2019

friedmud commented Jun 29, 2019 via email

dschwen commented Jun 29, 2019

Hysrok commented Jul 4, 2019 • edited

permcody commented Jul 8, 2019 via email

permcody commented Jul 9, 2019

moosebuild commented Jul 11, 2019

permcody commented Jul 11, 2019

permcody commented Jul 11, 2019

moosebuild commented Jul 11, 2019 • edited

permcody left a comment

Choose a reason for hiding this comment

dschwen Jul 16, 2019

Choose a reason for hiding this comment

permcody Jul 16, 2019

Choose a reason for hiding this comment

permcody commented Jul 16, 2019

friedmud commented Jul 23, 2019 via email

Hysrok commented Jul 4, 2019 •

edited

moosebuild commented Jul 11, 2019 •

edited