Hi Matt,

I support this change. I think it'll be nicer not having to compare values that are ~1e24 or ~1e-24, etc. The only thing that comes to mind is that testing in typically CGS invokes the comoving to proper cosmology reference frame transform, so we should make sure that doesn't get lost in the shuffle.

Britton

On Wed, Apr 29, 2020 at 3:52 AM Matthew Turk <matthewturk@gmail.com> wrote:
Hi folks,

In going through the yt4-merge pull request, Kacper and I tracked down
some issues related to the changeover in unyt from using reference
values in CGS to using reference values in MKS.  Rather than attempt
to reconcile these two things (ok actually we *tried* but it turns out
it's super duper hard, thanks a LOT floating point math) it seems that
the simpler thing to do is to make most of the values compared in the
base unit system of "code" rather than CGS.

I have issued this pull request:

https://github.com/yt-project/yt/pull/2568

to change over the default for the answer value tests to use
unit_system="code" if the ds is supplied as a string (rather than an
object).

Now, if we are concerned that we do not have adequate test coverage
for verifying that the units are in CGS, or if we have concerns about
this glossing over some CGS conversion factor issues, we should
perhaps supplement these tests.

This PR will be ready once all of the test storage IDs have been
updated, and once I've migrated most/all of the datasets to using
unit_system="code", but I wanted to open this up for possible
discussion here.  It's entirely possible this is not an acceptable
solution, and I am hoping that folks may have some feedback.  What am
I missing?  :)

Thanks,

Matt

PS I genuinely think that this might un-clog a whole bunch of issues
in the yt4-merge, and we can focus on just verifying the results for
the SPH datasets.  Hooray!
_______________________________________________
yt-dev mailing list -- yt-dev@python.org
To unsubscribe send an email to yt-dev-leave@python.org
https://mail.python.org/mailman3/lists/yt-dev.python.org/