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

Add xsplot output reader #95

Merged
merged 27 commits into from
Apr 20, 2018
Merged

Conversation

gridley
Copy link
Contributor

@gridley gridley commented Mar 13, 2018

Addresses #15

There's a reader which inherits from BaseReader to parse the output from Serpent's xsplot output. This includes material total macroscopic cross sections, constituent microscopic cross sections, and finally nu values for fissionable isotopes. Stuff gets put into a dictionary of XSData instances which inherit from NamedObject, with keys corresponding to intuitive object names. The XSData instances have numpy arrays for numeric cross sections values, and corresponding MT numbers and MT number descriptions to describe columns in the numeric XS array.

I have started on a test for this, but it's mainly just copied from the detector test and is yet to be fixed.

There's still some work to do on adding methods to easily plot some of these things.

@drewejohnson drewejohnson self-requested a review March 13, 2018 11:57
@drewejohnson drewejohnson self-assigned this Mar 13, 2018
@drewejohnson drewejohnson added this to the 1.0 milestone Mar 13, 2018
Copy link
Collaborator

@drewejohnson drewejohnson left a comment

Choose a reason for hiding this comment

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

This looking sweet! I know this is a work in progress, but I wanted to get out in front with some advice/comments

Things blocking review

  1. Review comments
  2. Unittests
  3. Plotting
  4. Documentation - as jupyter notebook and as rst

Section 4.4.2 in the serpentTools documentation shows a quick way to convert a jupyter notebook to the reStructuredText needed for sphinx

self.isIso = True

# serpent starts their names with an "m" if it's a whole-material XS
self.isIso = isIso
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why the double declaration?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

whoops! To really make sure it's declared, of course. Thanks for catching this.

self.hasNuData = False

@staticmethod
def negativeMTDescription(mt):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could this instead just be a class attribute, i.e.

class XSData(----)
    NEGATIVE_MTS = {}

Saves the need to rebuild/redeclare this every time the method is called.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The code is 110% cleaner due to this suggestion. thanks.


if not isinstance(self.xsdata, np.ndarray):
return False
if self.MT == []:
Copy link
Collaborator

Choose a reason for hiding this comment

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

empty lists automatically evaluate to False, so this can be replaced with if not self.MT

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hella pythonic, thanks.

if self.MT == []:
return False
else:
return True
Copy link
Collaborator

Choose a reason for hiding this comment

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

The else statement is redundant, because the return True will be automatically evaluated if the second if statement is False

Copy link
Contributor Author

Choose a reason for hiding this comment

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

factual!

@@ -0,0 +1,117 @@
"""Parser responsible for reading the ``*dep.m`` files"""
Copy link
Collaborator

Choose a reason for hiding this comment

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

Update

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Haha, whoops.

metadata: dict
misc stuff
"""
__doc__ = __doc__.format(attrs=docAttrs)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please see the above comment about doc strings

info('Done reading xsplot file')
debug(' found {} xs listings'.format(len(self.xsections)))

def _precheck(self):
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

Attributes
----------

""".format(params=docParams)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not just directly add docParams to the docstring?

This approach is useful for deeper levels of abstraction like with the DetectorBase, Detector, and SampledDetectors from #93. If you intend on adding an XsPlotSampler to this PR* then this would absolutely make sense. Here, not a ton of benefit

*The approval of this PR is not dependent on adding such a sampler

Copy link
Contributor Author

@gridley gridley Mar 24, 2018

Choose a reason for hiding this comment

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

Yeah, I don't know. That's what I would have rather done, but I didn't know if you guys preferred to do docstrings like that for some reason, since that syle is present in a few other files.

On the XSPlotSampler, could you explain the significance of such a thing? I see that the "SampledDetector" can compute RMS error on the detector readings, and spread of error on detector readings. What exactly is this used for?

By the way, unfortunately, in the _xsplot output, no uncertainty data gets provided.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The reformatting of the docstrings using these class attribtues helps reduce the amount of retyping code when writing docstrings for subclasses. For example, the Detector and SampledDetector are basically the same object, just the latter utilizes data from many Detector instances as has some minor method tweaks. The class docstrings, listing the attributes of the two, are very similar, so I wrote in the docAttrs on the DetectorBase to easily list the shared attributes.

This isn't something that is beneficial for a one off container. If we eventually write an XsPlotSampler, then this type of docstring formatting would be useful. I don't know how beneficial an XsPlotSampler would be, to be honest. I'm not sure how widely used this feature is within the SERPENT community.

@gridley
Copy link
Contributor Author

gridley commented Mar 24, 2018

Aight, I got some testing in now. Sorry about the slow progress, you know how this time in the semester tends to go.

@drewejohnson
Copy link
Collaborator

I'll take a look why the py 3.6 build is failing and do a more complete review later today.
No need to explain the end of semester, right there with you 😆

Copy link
Collaborator

@drewejohnson drewejohnson left a comment

Choose a reason for hiding this comment

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

Thank you for implementing the changes from the last review. The code looks a lot cleaner now.

My biggest issue with this PR now is the use of pickle in the unit tests. I know our unit tests can be improved, but I'm not a huge fan of distributing and using pickled objects when we can't verify and validate their contents.

Things blocking review

  1. Address the pickling situation
  2. Documentation - jupyter and rst
  3. Successful CI build - use paths relative to TEST_ROOT to find files
  4. Plotting?


Attributes
----------
{attrs:s}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Either pass in something to format this, or explicitly write the attributes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

got it!

@@ -45,7 +47,8 @@
r'(.*_det\d+\.m)': DetectorReader,
r'(.*_res\.m)': ResultsReader,
r'(.*_fmtx\d+\.m)': FissionMatrixReader,
r'(.*\.bumat\d+)': BumatReader
r'(.*\.bumat\d+)': BumatReader,
r'(.*_xs[0-9]*\.m)' : XSPlotReader
Copy link
Collaborator

Choose a reason for hiding this comment

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

Minor tweak: replace [0-9]* with \d* for the same effect with one less character lol

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All my life, I've used [0-9] and not \d. Thanks for the enlightenment.


else:
print(chunk)
error('Unidentifiable entry {}'.format(chunk[0]))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we want the whole chunk printed here? And should this be a more severe, perhaps fatal error?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, I don't think it should be an Exception or fatal error. Here's why: some strange XS may not be identified. One I came across the other day was i11023_06c_bra_f, which seems to be constant and uninteresting, somehow related to n,gamma. Maybe some sort of branching to an excited state.

There's probably some moreXS like i11023_06c_bra_f I didn't catch. So, without raising the exception, this should just skip past the chunk and continue to report only the expected stuff. As I come across these, I'll keep adding in support. Eventually I'll just sort through the source and get them all logged, but that's a bit much time for now.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Got it got it.

Would you be able to still store this array, perhaps in an extras or others dictionary? The reason is that while it may be uninteresting to us, there may be someone who really wants this information. We want to store all of the information present in the file. This way, if people use this reader a lot and we find out that i11023_06c_bra_f is used a lot, then we can properly treat it.

If you want, you can add some settings that would filter out the values stores. This is up to you though.

import os
import unittest
import numpy
import pickle
Copy link
Collaborator

Choose a reason for hiding this comment

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

I have mixed feelings about pickling. On one hand, your tests are super simple compared to directly comparing arrays that are hand written into the test example: depletion tester.

On the otherhand, pickling is kind of closed off and potentially dangerous. When unpickling something, there is the chance to execute malicious code. The pickle module documentation gives:

Warning : The pickle module is not secure against erroneous or maliciously constructed data. Never unpickle data received from an untrusted or unauthenticated source.

We don't have any real way of knowing what's really in the pickle, aside from some of it can be used to pass this unit test. If we can come up with an open and reliable way to prove that there is nothing malicious, then pickling would be the way to go.

Therefore for now, please remove the use of the pickle module. If you have ideas on improving the unit test system, please add them here and I will be happy to discuss alternatives

if not self.MT:
return False

return True
Copy link
Collaborator

Choose a reason for hiding this comment

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

Any plans to implement plotting?

Copy link
Contributor Author

@gridley gridley Mar 25, 2018

Choose a reason for hiding this comment

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

yes, getting automated plotting in is an absolute must, IMO. I'm also planning to add some automatic LaTeX table generation for few group stuff in reports. Maybe table generation like that would go better in its own serpentTools class though. Hm.

Copy link
Collaborator

Choose a reason for hiding this comment

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

LaTeX tabling would be awesome. For generality, we could utilize something like pandas.DataFrame.to_latex or a jinja2 template.

I do want to do more with the templating system given with jinja, like for writing materials and maybe cross sections for nodal diffusion. For now, since this is a one-off application, I don't think we need a dedicated class/module for this, not yet at least. But I am open to the idea in the future

@gridley
Copy link
Contributor Author

gridley commented Mar 25, 2018

Hey Drew, thanks for the latest tips. I'll return to this on Wednesday or so to finish the thing off. I do have comments on the pickling though. Because using pickles for unit tests simplifies stuff so much, do you think that checking the pickle hashsum would be acceptable? I think this would be kind of novel and fun. Since nobody is running serpentTools on anything even resembling a listening server, I imagine we regardless wouldn't need to worry about exploits like this.

@drewejohnson
Copy link
Collaborator

I'm not sure about using a hash or checksum or something similar. We would need a very open procedure for generating these objects that can be verified by any end user. The downside of using any serialized reader (pickle, JSON, yaml, etc) is that you really have to execute/load the potentially malicious object before using it. I played around with yaml a bit and it yields a nice 5000+ line text file with some binary artifacts but it can dump and then load a valid reader, with all its objects in tact.

Pickling can be used for a lot more than just listening servers. The unpickling process can execute arbitrary commands, using functions like exec and subprocess.POpen to really do some damange.

For now, I would ask that the current framework be mimiced in your testing. While it may be tedious, it is open and can be verified easily. I welcome and look forward your thoughts on improving the unit testing process

@gridley
Copy link
Contributor Author

gridley commented Mar 30, 2018

Aight, fair enough. It'll take longer for me to get the PR done though.

@gridley
Copy link
Contributor Author

gridley commented Apr 4, 2018

Still working on this, sorry. Plotting looks good, but tests and examples are yet to be done.

@@ -65,6 +64,24 @@ def __init__(self, name, metadata, isIso=False):
# whether nu data present for fissionables
self.hasNuData = False

@staticmethod
def starify(arr1, arr2):
Copy link
Collaborator

Choose a reason for hiding this comment

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

This just makes a stair step plot, right? If so, you may want to checkout the drawstyle kwarg for matplotlib.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

AH! I've been looking for something like this forever. In Mathematica, it's called InterpolationOrder. I tried searching for something similar in MPL but never had luck. Thanks for pointing this out! :D

Copy link
Collaborator

Choose a reason for hiding this comment

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

Happy to help 😀

@@ -114,3 +131,39 @@ def hasExpectedData(self):
return False

return True

def plot(self, mts, **kwargs):
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would recommend looking at some of the other plot methods we have and try and implement a similar approach, mainly with some of the formatting arguments like autolabeling and axis scaling. The magicPlotDecorator may be a useful guide for

  1. a consistent set of arguments and,
  2. minimal docstring writing for various kwargs 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, will do, Drew!

@gridley
Copy link
Contributor Author

gridley commented Apr 9, 2018

OK, all done! Anything else?

Copy link
Collaborator

@drewejohnson drewejohnson left a comment

Choose a reason for hiding this comment

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

Really great work! Thank you for addressing my comments in previous pulls.

Things blocking approval

  1. Resolve conflicts with develop
  2. Address comments regarding plot method
  3. Unit test for list/show/printMT method
  4. Incorporate XSPlot.rst into documentation directory, not examples

Main comments

Plotting

I really like the outputs from the plotting routine you've implemented. Please utilize the keywords that are detected by the @magicPlotDocDecorator so that

  1. there is a high level of control for the plotting without having to know the details of matplotlib
  2. an instance of pyplot.Axes is returned from the method. I included some rationale in the comment
  3. Data is plotted using ax.plot and then scaled according to the input arguments

While only having a loglog options works well most of the time, when you have data that doesn't span orders of magnitude, you lose some of the plotted data, ex: scatter and n,2n

Documentation/Example

The example looks really great. Plotting is super simple and crisp. Can you include a view of accessing the actual arrays as well? How would I, the uninformed end-user, access some of the cross sections that are stored?

The XSPlot.rst needs to be moved into docs/examples and included in docs/examples/index.rst to be fully included in the online documentation.
Also, add a brief autoclass-ed XSPlotReader and container files in docs/api - example with detector reader and detector container. Be sure to include the new files in their respective indexes.

Updates

Be sure to merge in recent changes from develop and resolve conflicts in serpentTools/__init__.py

elif 'bra_f' in chunk[0]:
warning("There is this weird 'bra_f' XS. these seem to be"
" constant. skipping though.")

Copy link
Collaborator

Choose a reason for hiding this comment

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

Just getting around to seeing this, but can you instead add this data to a new dictionary? While it may appear unimportant, we do want to store all the data in the outputs. We can add settings to filter this out later, or properly treat the data with more insight from the community

@@ -132,20 +115,45 @@ def hasExpectedData(self):

return True

def plot(self, mts, **kwargs):
@magicPlotDocDecorator
def plot(self, mts, logscale=True, figargs={}, **kwargs):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please implement the following keyword arguments and their functionality:

  • loglog instead of logscale - set both axis to logarithmic
  • logx - set the x axis to log if not loglog
  • logy - set the y axis to log if not loglog
  • ax - plot on which to draw the plots, i.e. ax.plot(<x>, <y>). This allows the plots to be added to an existing plot if given, or create a fresh plot. This should also be returned from the method. Reason to not create/return a new figure: this approach integrates very well with subplots, and axis objects are more explicit to work with. Figures can be obtained with pyplot.gcf() if the user really wants to. In my experience, I really only interact directly with the figure if I have multiple figures created and I want to save a specific figure. All other functions can be obtained by interacting with the ax on the figure, or with pyplot.savefig().

When adding these keyword arguments into the call signature, simply add their shortcut name, i.e. {loglog}, into the docstring. The magicPlotDocDecorator updates the docstring with a proper explanation of what these common arguments do.

In the future, we may include another decorator that does the logscalling after the fact, which would reduce the amount of times we have to manually write in the scaling.

In doing so, remove the figargs dict, as the raw figure will not be created.


# convert to list if it's just one MT
if isinstance(mts, int):
if mts == 'all':
Copy link
Collaborator

Choose a reason for hiding this comment

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

Perhaps let mts have a default of None, so that someone can simply do obj.plot() and all the mts are plotted and labeled

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done!

"a list of integer MTs, or a single interger"
"instead, {} of type {} was passed.".format(
mts, type(mts)))
raise Exception(msg)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this be a TypeError rather than a blanket Exception? More descriptive that way.
Either way, can you add a Raises section to the docstring indicating under what circumstances this error is raised?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

excellent point. Exception is so main stream.


for mt in mts:
if mt not in self.MT:
error("{} not in collected MT numbers, {}".format(mt, self.MT))
fig = pyplot.figure(**kwargs)
fig = pyplot.figure(**figargs)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Replace with ax=ax or pyplot.axes() per main comment above

Copy link
Contributor Author

Choose a reason for hiding this comment

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

got it!

return XSData.MTdescriptions[mt]
except KeyError:
error("Cannot find description for MT {}.".format(mt))
return None
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why should this method not just be return XSData.MTdescriptions[mt]? Key errors will be raised, and not suppressed. I understand the desire to ensure that the value is indeed negative, but this seems like this could just be two steps: check that the key is indeed negative/raise error otherwise, and then access the description from the dictionary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

excellent point.

def _read(self):
"""Read through the depletion file and store requested data."""
info('Preparing to read {}'.format(self.filePath))
keys = ['E', 'i[0-9]{4,5}', 'm[a-zA-Z]']
Copy link
Collaborator

Choose a reason for hiding this comment

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

  • [0-9] ➡️ \d
  • [a-zA-Z] ➡️ [a-Z]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

wow, bringing down my character count by this much will make it look like I don't write much code. 😄


elif 'bra_f' in chunk[0]:
warning("There is this weird 'bra_f' XS. these seem to be"
" constant. skipping though.")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Repeating from the PR comment thread: add these to a separate dictionary so that all data is indeed stored. If we want to add settings to filter this out later, then we can do that. It may not seem important, but it's in the output for a reason

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

"The incomplete XS is {}".format(xs.name))

# check energy grid found
assert 'egrid' in self.metadata.keys()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add an error message to this assertion

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

ax.legend(descriplist)
ax.set_title('{} cross section{}'.format(self.name,'s' if len(mts)>1 else ''))
ax.set_xlabel('Energy (MeV)')
ax.set_ylabel('XS ({})'.format('b' if self.isIso else 'cm$^{-1}$'))
Copy link
Collaborator

Choose a reason for hiding this comment

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

To be a little more formal, replace with Cross section, rather than XS

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

@gridley
Copy link
Contributor Author

gridley commented Apr 11, 2018

Note to self: show how to expose the XS data np array in the jupyter examples. Going to come back to this after senior design is done.

@gridley
Copy link
Contributor Author

gridley commented Apr 19, 2018

@drewejohnson hey, regarding the addition of linear scale plots, I'm pretty sure this is unneeded. You mentioned n.2n and scattering scaling on different scales, but this effect that appears in the plot with n,2n is due to it occurring only above around a 1 MeV threshold. I'll add them anyways though! Aside from that, I'll be working on this some tonight.

@drewejohnson
Copy link
Collaborator

Stellar! For your next push, be sure to merge changes from develop into your branch and resolve the merge conflicts. This is what is causing the Travis build to stall - source

@gridley
Copy link
Contributor Author

gridley commented Apr 19, 2018

OK, what now?

Copy link
Collaborator

@drewejohnson drewejohnson left a comment

Choose a reason for hiding this comment

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

Thank you for addressing my comments! I really love the tabulate method you added too 🏆

(Minor) Items blocking merge

  1. Merge latest changes from develop
  2. Include example into docs/examples/index.rst
  3. Include class documentation in docs/api
  4. Brief unit test for printMT method

Discretionary

  1. Add unit test for tabulate method - May have to include pandas as a requirement for travis build

Notes

As a general guideline, all public methods should have unit tests so that we can ensure things work as the project progresses. While the tabulate and printMT method are more for quick inspections, I still believe they deserve sufficient unit tests. Since you already have done the full test on the Reader and container, the tests should be straight forward.

Since you have the rst documentation written, all that is needed for it to be included in the manual is to include the file in docs/examples/index.rst. This way people can see the full documentation online. Also add the reader and container to the docs/api section using sphinx autoclass/automethod commands.

import pandas as pd
except ModuleNotFoundError:
print("This requires the pandas package.")
return None
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would you rather just let the import error be raised? It's in the docstring, so one thing needs to be updated for consistency

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair enough.

try:
assert 'egrid' in self.metadata.keys()
except AssertionError as e:
e.args += 'No energy grid found in xsplot parser.'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Will this raise the AssertionError or simply append to args?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This prints additonal info with the AssertionError. Reference: https://stackoverflow.com/questions/3807694/how-to-change-the-message-in-a-python-assertionerror

@@ -115,8 +111,80 @@ def hasExpectedData(self):

return True

def tabulate(self, mts='all', colnames=None):
Copy link
Collaborator

Choose a reason for hiding this comment

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

I really like this! The DataFrames are great for visualization! Plus, those can go straight to LaTeX should the user want too 💯


# could possibly automatically set up subplotting
ax = fig.add_subplot(111)
# as per serpentTools style! (so pythonic!)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Lol while this is True, please remove

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok ☹️

@gridley
Copy link
Contributor Author

gridley commented Apr 20, 2018

OK, should be polished off now.

@drewejohnson
Copy link
Collaborator

Add pandas as an install for travis

@gridley
Copy link
Contributor Author

gridley commented Apr 20, 2018

Gonna be honest, there was a slight delay because I had to check for the github panda emoji code.

drewejohnson and others added 20 commits April 20, 2018 15:30
Added some scripts and documentation that explain the default settings 
and variable groups.
Scripts convert the default settings and variable groups into something 
that can be well understood by the sphinx documentation builder for
making nice doc pages. Settings and groups are also hyperlinked so 
that they can be referenced from other parts of the documentation.
Fixes CORE-GATECH-GROUP#113

If a branch is found to have only a single name, then the
name of the branch, and key in the branches dictionary, is taken
to be the first entry. The previous implementation would place
these branches under a single entry tuple, which required
extraneous keystrokes for retrieval. Now, the string name of the
branch is the key.

BranchReader still supports accessing branches with single entry
tuples, but this feature will be removed in the next release. A 
future warning is implemented if such a branch is accessed using
this old manner, i.e. [('nom'), ] vs. ['nom']

Also fixed a minor bug on the homogenized universe container, where
the b1Kinf data would be stored in the infinite medium dictionaries.
Fixes CORE-GATECH-GROUP#119

If the value of timePoints is not given to either
plot or spreadPlot for SampledDepletedMaterial,
the default x-points are taken according to the
value of xUnits passed into the method.

Added some better x-axis formatting for
SampledDepletedMaterial and DepletedMaterial containers
…P#118)

Changes include adding 2.1.30 into the serpentVersions settings
options and adding the diffusion variable set for 2.1.30 into
variables.yaml

Updated the default settings documentation to include 2.1.30

Added some documentation regarding what changes need to be made
in order to include support for additional versions. This
will be updated as issue CORE-GATECH-GROUP#117 is addressed and a proper
procedure for identifying what variables are new/modified
across versions

Bump changelog for 0.3.1 release
If a globbed argument was passed into a Sampler
constructor, then no files would be loaded onto
the object. This would not cause any problems during
the read process, until now.

Two changes included in this commit. One, during
the extendFiles function, if a file glob
does not return any files, a warning is raised
indicating as much. Second, following this function call,
if no files are to be stored on the sampler and to be
@gridley
Copy link
Contributor Author

gridley commented Apr 20, 2018

OK, maybe, just maybe, that rebase fixed it.

@drewejohnson drewejohnson merged commit bf9e16d into CORE-GATECH-GROUP:develop Apr 20, 2018
@gridley gridley deleted the xsplotter branch April 20, 2018 20:09
DanKotlyar added a commit that referenced this pull request May 9, 2018
Reverted to  "Add xsplot output reader (#95)"

Keeping files directly related to this branch/PR including

test_ResultsReader.py
pwr_res*.m
ResultsReader.ipynb
results.py
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.

2 participants